* [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called
@ 2012-07-17 21:07 Paul Moore
2012-07-17 21:24 ` Paul Moore
2012-07-18 16:02 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Paul Moore @ 2012-07-17 21:07 UTC (permalink / raw)
To: netdev
As reported by Alan Cox, and verified by Lin Ming, when a user
attempts to add a CIPSO option to a socket using the CIPSO_V4_TAG_LOCAL
tag the kernel dies a terrible death when it attempts to follow a NULL
pointer (the skb argument to cipso_v4_validate() is NULL when called via
the setsockopt() syscall).
This patch fixes this by first checking to ensure that the skb is
non-NULL before using it to find the incoming network interface. In
the unlikely case where the skb is NULL and the user attempts to add
a CIPSO option with the _TAG_LOCAL tag we return an error as this is
not something we want to allow.
A simple reproducer, kindly supplied by Lin Ming, although you must
have the CIPSO DOI #3 configure on the system first or you will be
caught early in cipso_v4_validate():
#include <sys/types.h>
#include <sys/socket.h>
#include <linux/ip.h>
#include <linux/in.h>
#include <string.h>
struct local_tag {
char type;
char length;
char info[4];
};
struct cipso {
char type;
char length;
char doi[4];
struct local_tag local;
};
int main(int argc, char **argv)
{
int sockfd;
struct cipso cipso = {
.type = IPOPT_CIPSO,
.length = sizeof(struct cipso),
.local = {
.type = 128,
.length = sizeof(struct local_tag),
},
};
memset(cipso.doi, 0, 4);
cipso.doi[3] = 3;
sockfd = socket(AF_INET, SOCK_DGRAM, 0);
#define SOL_IP 0
setsockopt(sockfd, SOL_IP, IP_OPTIONS,
&cipso, sizeof(struct cipso));
return 0;
}
CC: Lin Ming <mlin@ss.pku.edu.cn>
Reported-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
net/ipv4/cipso_ipv4.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index c48adc5..667c1d4 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1725,8 +1725,10 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
case CIPSO_V4_TAG_LOCAL:
/* This is a non-standard tag that we only allow for
* local connections, so if the incoming interface is
- * not the loopback device drop the packet. */
- if (!(skb->dev->flags & IFF_LOOPBACK)) {
+ * not the loopback device drop the packet. Further,
+ * there is no legitimate reason for setting this from
+ * userspace so reject it if skb is NULL. */
+ if (skb == NULL || !(skb->dev->flags & IFF_LOOPBACK)) {
err_offset = opt_iter;
goto validate_return_locked;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called
2012-07-17 21:07 [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called Paul Moore
@ 2012-07-17 21:24 ` Paul Moore
2012-07-17 21:31 ` David Miller
2012-07-18 16:02 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Paul Moore @ 2012-07-17 21:24 UTC (permalink / raw)
To: netdev
On Tuesday, July 17, 2012 05:07:47 PM Paul Moore wrote:
> As reported by Alan Cox, and verified by Lin Ming, when a user
> attempts to add a CIPSO option to a socket using the CIPSO_V4_TAG_LOCAL
> tag the kernel dies a terrible death when it attempts to follow a NULL
> pointer (the skb argument to cipso_v4_validate() is NULL when called via
> the setsockopt() syscall).
>
> This patch fixes this by first checking to ensure that the skb is
> non-NULL before using it to find the incoming network interface. In
> the unlikely case where the skb is NULL and the user attempts to add
> a CIPSO option with the _TAG_LOCAL tag we return an error as this is
> not something we want to allow.
...
> CC: Lin Ming <mlin@ss.pku.edu.cn>
> Reported-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Argh, I just realized I forgot to CC the stable folks.
David, if you don't queue this up for them, let me know and I'll resend it to
stable once it hits Linus' tree.
> ---
> net/ipv4/cipso_ipv4.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index c48adc5..667c1d4 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1725,8 +1725,10 @@ int cipso_v4_validate(const struct sk_buff *skb,
> unsigned char **option) case CIPSO_V4_TAG_LOCAL:
> /* This is a non-standard tag that we only allow for
> * local connections, so if the incoming interface is
> - * not the loopback device drop the packet. */
> - if (!(skb->dev->flags & IFF_LOOPBACK)) {
> + * not the loopback device drop the packet. Further,
> + * there is no legitimate reason for setting this from
> + * userspace so reject it if skb is NULL. */
> + if (skb == NULL || !(skb->dev->flags & IFF_LOOPBACK)) {
> err_offset = opt_iter;
> goto validate_return_locked;
> }
--
paul moore
security and virtualization @ redhat
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called
2012-07-17 21:24 ` Paul Moore
@ 2012-07-17 21:31 ` David Miller
2012-07-18 13:03 ` Paul Moore
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2012-07-17 21:31 UTC (permalink / raw)
To: pmoore; +Cc: netdev
From: Paul Moore <pmoore@redhat.com>
Date: Tue, 17 Jul 2012 17:24:50 -0400
> David, if you don't queue this up for them, let me know and I'll resend it to
> stable once it hits Linus' tree.
I will be sure to queue it up to -stable when I apply it, as I always
do for appropriate patches.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called
2012-07-17 21:31 ` David Miller
@ 2012-07-18 13:03 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2012-07-18 13:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Tuesday, July 17, 2012 02:31:37 PM David Miller wrote:
> From: Paul Moore <pmoore@redhat.com>
> Date: Tue, 17 Jul 2012 17:24:50 -0400
>
> > David, if you don't queue this up for them, let me know and I'll resend it
> > to stable once it hits Linus' tree.
>
> I will be sure to queue it up to -stable when I apply it, as I always
> do for appropriate patches.
Okay, thanks. I just wanted to make sure it hit -stable one way or another.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called
2012-07-17 21:07 [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called Paul Moore
2012-07-17 21:24 ` Paul Moore
@ 2012-07-18 16:02 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2012-07-18 16:02 UTC (permalink / raw)
To: pmoore; +Cc: netdev
From: Paul Moore <pmoore@redhat.com>
Date: Tue, 17 Jul 2012 17:07:47 -0400
> As reported by Alan Cox, and verified by Lin Ming, when a user
> attempts to add a CIPSO option to a socket using the CIPSO_V4_TAG_LOCAL
> tag the kernel dies a terrible death when it attempts to follow a NULL
> pointer (the skb argument to cipso_v4_validate() is NULL when called via
> the setsockopt() syscall).
>
> This patch fixes this by first checking to ensure that the skb is
> non-NULL before using it to find the incoming network interface. In
> the unlikely case where the skb is NULL and the user attempts to add
> a CIPSO option with the _TAG_LOCAL tag we return an error as this is
> not something we want to allow.
>
> A simple reproducer, kindly supplied by Lin Ming, although you must
> have the CIPSO DOI #3 configure on the system first or you will be
> caught early in cipso_v4_validate():
...
> CC: Lin Ming <mlin@ss.pku.edu.cn>
> Reported-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
Applied and queued up for -stable, thanks Paul.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-18 16:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 21:07 [PATCH] cipso: don't follow a NULL pointer when setsockopt() is called Paul Moore
2012-07-17 21:24 ` Paul Moore
2012-07-17 21:31 ` David Miller
2012-07-18 13:03 ` Paul Moore
2012-07-18 16:02 ` David Miller
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.