2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 06:15:47 +00:00

tc: Fix crash on malformed reply from kernel.

The tc module combines the use of the `tc_transact` helper
function for communication with the in-kernel tc infrastructure
with assertions on the reply data by `ofpbuf_at_assert` on the
received data prior to further processing.

With the presence of bugs on the kernel side, we need to treat
the kernel as an unreliable service provider and replace assertions
on the reply from it with checks to avoid a fatal crash of OVS.

For the record, the symptom of the crash is this in the log:
  EMER|include/openvswitch/ofpbuf.h:194:
      assertion offset + size <= b->size failed in ofpbuf_at_assert()

And an excerpt of the backtrace looks like this:
  ofpbuf_at_assert (offset=16, size=20) at include/openvswitch/ofpbuf.h:194
  tc_replace_flower  at lib/tc.c:3223
  netdev_tc_flow_put at lib/netdev-offload-tc.c:2096
  netdev_flow_put    at lib/netdev-offload.c:257
  parse_flow_put     at lib/dpif-netlink.c:2297
  try_send_to_netdev at lib/dpif-netlink.c:2384

Reported-At: https://launchpad.net/bugs/2018500
Fixes: 5c039ddc64 ("netdev-linux: Add functions to manipulate tc police action")
Fixes: e7f6ba220e ("lib/tc: add ingress ratelimiting support for tc-offload")
Fixes: f98e418fbd ("tc: Add tc flower functions")
Fixes: c1c9c9c4b6 ("Implement QoS framework.")
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Frode Nordahl
2023-06-06 20:33:35 +02:00
committed by Ilya Maximets
parent 64cdc290ef
commit 106ef21860
2 changed files with 59 additions and 24 deletions

View File

@@ -2714,8 +2714,16 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t kbits_rate,
err = tc_transact(&request, &reply);
if (!err) {
struct tcmsg *tc =
ofpbuf_at_assert(reply, NLMSG_HDRLEN, sizeof *tc);
struct ofpbuf b = ofpbuf_const_initializer(reply->data, reply->size);
struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg);
struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc);
if (!nlmsg || !tc) {
VLOG_ERR_RL(&rl,
"Failed to add match all policer, malformed reply");
ofpbuf_delete(reply);
return EPROTO;
}
ofpbuf_delete(reply);
}
@@ -5744,26 +5752,27 @@ static int
tc_update_policer_action_stats(struct ofpbuf *msg,
struct ofputil_meter_stats *stats)
{
struct ofpbuf b = ofpbuf_const_initializer(msg->data, msg->size);
struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg);
struct tcamsg *tca = ofpbuf_try_pull(&b, sizeof *tca);
struct ovs_flow_stats stats_dropped;
struct ovs_flow_stats stats_hw;
struct ovs_flow_stats stats_sw;
const struct nlattr *act;
struct nlattr *prio;
struct tcamsg *tca;
int error = 0;
if (!stats) {
goto exit;
}
if (NLMSG_HDRLEN + sizeof *tca > msg->size) {
if (!nlmsg || !tca) {
VLOG_ERR_RL(&rl, "Failed to get action stats, size error");
error = EPROTO;
goto exit;
}
tca = ofpbuf_at_assert(msg, NLMSG_HDRLEN, sizeof *tca);
act = nl_attr_find(msg, NLMSG_HDRLEN + sizeof *tca, TCA_ACT_TAB);
act = nl_attr_find(&b, 0, TCA_ACT_TAB);
if (!act) {
VLOG_ERR_RL(&rl, "Failed to get action stats, can't find attribute");
error = EPROTO;
@@ -6028,20 +6037,26 @@ static int
tc_parse_class(const struct ofpbuf *msg, unsigned int *handlep,
struct nlattr **options, struct netdev_queue_stats *stats)
{
struct ofpbuf b = ofpbuf_const_initializer(msg->data, msg->size);
struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg);
struct tcmsg *tc = ofpbuf_try_pull(&b, sizeof *tc);
static const struct nl_policy tca_policy[] = {
[TCA_OPTIONS] = { .type = NL_A_NESTED, .optional = false },
[TCA_STATS2] = { .type = NL_A_NESTED, .optional = false },
};
struct nlattr *ta[ARRAY_SIZE(tca_policy)];
if (!nl_policy_parse(msg, NLMSG_HDRLEN + sizeof(struct tcmsg),
tca_policy, ta, ARRAY_SIZE(ta))) {
if (!nlmsg || !tc) {
VLOG_ERR_RL(&rl, "failed to parse class message, malformed reply");
goto error;
}
if (!nl_policy_parse(&b, 0, tca_policy, ta, ARRAY_SIZE(ta))) {
VLOG_WARN_RL(&rl, "failed to parse class message");
goto error;
}
if (handlep) {
struct tcmsg *tc = ofpbuf_at_assert(msg, NLMSG_HDRLEN, sizeof *tc);
*handlep = tc->tcm_handle;
}