| Message ID | 151520106487.32271.6013001625427346680.stgit@dwillia2-desk3.amr.corp.intel.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | prevent bounds-check bypass via speculative execution | expand |
On 1/6/2018 4:11 AM, Dan Williams wrote: > Static analysis reports that 'offset' may be a user controlled value > that is used as a data dependency reading from a raw6_frag_vec buffer. > In order to avoid potential leaks of kernel memory values, block > speculative execution of the instruction stream that could issue further > reads based on an invalid '*(rfv->c + offset)' value. > > Based on an original patch by Elena Reshetova. > > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> > Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> > Cc: netdev@vger.kernel.org > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > net/ipv6/raw.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c > index 761a473a07c5..384e3d59d148 100644 > --- a/net/ipv6/raw.c > +++ b/net/ipv6/raw.c [...] > @@ -725,17 +726,17 @@ static int raw6_getfrag(void *from, char *to, int offset, int len, int odd, > struct sk_buff *skb) > { > struct raw6_frag_vec *rfv = from; > + char *rfv_buf; > > - if (offset < rfv->hlen) { > + if ((rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen))) { And here... > int copy = min(rfv->hlen - offset, len); > > if (skb->ip_summed == CHECKSUM_PARTIAL) > - memcpy(to, rfv->c + offset, copy); > + memcpy(to, rfv_buf, copy); > else > skb->csum = csum_block_add( > skb->csum, > - csum_partial_copy_nocheck(rfv->c + offset, > - to, copy, 0), > + csum_partial_copy_nocheck(rfv_buf, to, copy, 0), > odd); > > odd = 0; MBR, Sergei
On Fri, 05 Jan 2018 17:11:04 -0800 Dan Williams <dan.j.williams@intel.com> wrote: > Static analysis reports that 'offset' may be a user controlled value > that is used as a data dependency reading from a raw6_frag_vec buffer. > In order to avoid potential leaks of kernel memory values, block > speculative execution of the instruction stream that could issue further > reads based on an invalid '*(rfv->c + offset)' value. > > Based on an original patch by Elena Reshetova. > > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> > Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> > Cc: netdev@vger.kernel.org > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > net/ipv6/raw.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c > index 761a473a07c5..384e3d59d148 100644 > --- a/net/ipv6/raw.c > +++ b/net/ipv6/raw.c > @@ -33,6 +33,7 @@ > #include <linux/skbuff.h> > #include <linux/compat.h> > #include <linux/uaccess.h> > +#include <linux/compiler.h> > #include <asm/ioctls.h> > > #include <net/net_namespace.h> > @@ -725,17 +726,17 @@ static int raw6_getfrag(void *from, char *to, int offset, int len, int odd, > struct sk_buff *skb) > { > struct raw6_frag_vec *rfv = from; > + char *rfv_buf; > > - if (offset < rfv->hlen) { > + if ((rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen))) { > int copy = min(rfv->hlen - offset, len); Minor nit. Please don't do assignment in condition test here. Instead. rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen); if (rfv_buf) {
On Sat, Jan 6, 2018 at 6:48 AM, Stephen Hemminger <stephen@networkplumber.org> wrote: > On Fri, 05 Jan 2018 17:11:04 -0800 > Dan Williams <dan.j.williams@intel.com> wrote: > >> Static analysis reports that 'offset' may be a user controlled value >> that is used as a data dependency reading from a raw6_frag_vec buffer. >> In order to avoid potential leaks of kernel memory values, block >> speculative execution of the instruction stream that could issue further >> reads based on an invalid '*(rfv->c + offset)' value. >> >> Based on an original patch by Elena Reshetova. >> >> Cc: "David S. Miller" <davem@davemloft.net> >> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> >> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> >> Cc: netdev@vger.kernel.org >> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com> >> --- >> net/ipv6/raw.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c >> index 761a473a07c5..384e3d59d148 100644 >> --- a/net/ipv6/raw.c >> +++ b/net/ipv6/raw.c >> @@ -33,6 +33,7 @@ >> #include <linux/skbuff.h> >> #include <linux/compat.h> >> #include <linux/uaccess.h> >> +#include <linux/compiler.h> >> #include <asm/ioctls.h> >> >> #include <net/net_namespace.h> >> @@ -725,17 +726,17 @@ static int raw6_getfrag(void *from, char *to, int offset, int len, int odd, >> struct sk_buff *skb) >> { >> struct raw6_frag_vec *rfv = from; >> + char *rfv_buf; >> >> - if (offset < rfv->hlen) { >> + if ((rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen))) { >> int copy = min(rfv->hlen - offset, len); > > Minor nit. > > Please don't do assignment in condition test here. > Instead. > rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen); > if (rfv_buf) { Yeah, sorry about that. This was a hold over from an earlier version where nospec_array_ptr() did not include the necessary barrier and we relied on a new if_nospec helper, but now that if_nospec is no longer being proposed I can go back and clean this up.
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c index 761a473a07c5..384e3d59d148 100644 --- a/net/ipv6/raw.c +++ b/net/ipv6/raw.c @@ -33,6 +33,7 @@ #include <linux/skbuff.h> #include <linux/compat.h> #include <linux/uaccess.h> +#include <linux/compiler.h> #include <asm/ioctls.h> #include <net/net_namespace.h> @@ -725,17 +726,17 @@ static int raw6_getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb) { struct raw6_frag_vec *rfv = from; + char *rfv_buf; - if (offset < rfv->hlen) { + if ((rfv_buf = nospec_array_ptr(rfv->c, offset, rfv->hlen))) { int copy = min(rfv->hlen - offset, len); if (skb->ip_summed == CHECKSUM_PARTIAL) - memcpy(to, rfv->c + offset, copy); + memcpy(to, rfv_buf, copy); else skb->csum = csum_block_add( skb->csum, - csum_partial_copy_nocheck(rfv->c + offset, - to, copy, 0), + csum_partial_copy_nocheck(rfv_buf, to, copy, 0), odd); odd = 0;