Message ID | 20120717210738.22790.23522.stgit@sifl |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
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; > }
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }
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(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html