All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] openvswitch: potential NULL deref in sample()
@ 2012-07-23  7:46 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-07-23  7:46 UTC (permalink / raw)
  To: Jesse Gross
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller

If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
it leads to a NULL dereference when we call nla_len(acts_list).  This
is a static checker fix, not something I have seen in testing.

Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
This applies to Linus's tree.

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 48badff..c2351d6 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -325,6 +325,9 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
 		}
 	}
 
+	if (!acts_list)
+		return 0;
+
 	return do_execute_actions(dp, skb, nla_data(acts_list),
 						 nla_len(acts_list), true);
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [patch] openvswitch: potential NULL deref in sample()
@ 2012-07-23  7:46 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-07-23  7:46 UTC (permalink / raw)
  To: Jesse Gross
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller

If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
it leads to a NULL dereference when we call nla_len(acts_list).  This
is a static checker fix, not something I have seen in testing.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This applies to Linus's tree.

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 48badff..c2351d6 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -325,6 +325,9 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
 		}
 	}
 
+	if (!acts_list)
+		return 0;
+
 	return do_execute_actions(dp, skb, nla_data(acts_list),
 						 nla_len(acts_list), true);
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [patch] openvswitch: potential NULL deref in sample()
       [not found] ` <20120723074628.GA30892-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2012-07-23  8:00     ` David Miller
  2012-07-23 19:54     ` Jesse Gross
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2012-07-23  8:00 UTC (permalink / raw)
  To: dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

From: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Mon, 23 Jul 2012 10:46:28 +0300

> If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
> it leads to a NULL dereference when we call nla_len(acts_list).  This
> is a static checker fix, not something I have seen in testing.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Applied, thanks Dan.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch] openvswitch: potential NULL deref in sample()
@ 2012-07-23  8:00     ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2012-07-23  8:00 UTC (permalink / raw)
  To: dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Mon, 23 Jul 2012 10:46:28 +0300

> If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
> it leads to a NULL dereference when we call nla_len(acts_list).  This
> is a static checker fix, not something I have seen in testing.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied, thanks Dan.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch] openvswitch: potential NULL deref in sample()
       [not found] ` <20120723074628.GA30892-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2012-07-23 19:54     ` Jesse Gross
  2012-07-23 19:54     ` Jesse Gross
  1 sibling, 0 replies; 6+ messages in thread
From: Jesse Gross @ 2012-07-23 19:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller

On Mon, Jul 23, 2012 at 12:46 AM, Dan Carpenter
<dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
> it leads to a NULL dereference when we call nla_len(acts_list).  This
> is a static checker fix, not something I have seen in testing.
>
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

This can never happen in practice because the action list is validated
at the time that userspace installs the flow.  There are plenty of
things in this category that would appear to be unsafe because we'd
much rather do sanity checking on a per-flow basis rather than
per-packet.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch] openvswitch: potential NULL deref in sample()
@ 2012-07-23 19:54     ` Jesse Gross
  0 siblings, 0 replies; 6+ messages in thread
From: Jesse Gross @ 2012-07-23 19:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller

On Mon, Jul 23, 2012 at 12:46 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
> it leads to a NULL dereference when we call nla_len(acts_list).  This
> is a static checker fix, not something I have seen in testing.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

This can never happen in practice because the action list is validated
at the time that userspace installs the flow.  There are plenty of
things in this category that would appear to be unsafe because we'd
much rather do sanity checking on a per-flow basis rather than
per-packet.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-23 19:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-23  7:46 [patch] openvswitch: potential NULL deref in sample() Dan Carpenter
2012-07-23  7:46 ` Dan Carpenter
     [not found] ` <20120723074628.GA30892-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2012-07-23  8:00   ` David Miller
2012-07-23  8:00     ` David Miller
2012-07-23 19:54   ` Jesse Gross
2012-07-23 19:54     ` Jesse Gross

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.