| Message ID | 20180122112415.GA41074@beast |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | RDS: Fix rds-ping inducing kernel panic | expand |
On Mon, Jan 22, 2018 at 03:24:15AM -0800, Kees Cook wrote: > As described in: https://bugzilla.redhat.com/show_bug.cgi?id=822754 > > Attempting an RDS connection from the IP address of an IPoIB interface > to itself causes a kernel panic due to a BUG_ON() being triggered. > Making the test less strict allows rds-ping to work without crashing > the machine. > > A local unprivileged user could use this flaw to crash the sytem. s/sytem/system > > I think this fix was written by Jay Fenlason <fenlason@redhat.com>, > and extracted from the RedHat kernel patches here: > > https://oss.oracle.com/git/gitweb.cgi?p=redpatch.git;a=commitdiff;h=c7b6a0a1d8d636852be130fa15fa8be10d4704e8 > > This fix appears to have been carried by at least RedHat, Oracle, and > Ubuntu for several years. > > CVE-2012-2372 > > Reported-by: Honggang Li <honli@redhat.com> > Cc: stable@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > This is what I get for researching CVE lifetimes... > --- > net/rds/ib_send.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c > index 8557a1cae041..5fbf635d17cb 100644 > --- a/net/rds/ib_send.c > +++ b/net/rds/ib_send.c > @@ -506,7 +506,7 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm, > int flow_controlled = 0; > int nr_sig = 0; > > - BUG_ON(off % RDS_FRAG_SIZE); > + BUG_ON(!conn->c_loopback && off % RDS_FRAG_SIZE); > BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header)); To be honest this function full of BUG_ONs and it looks fishy to have them there. Why don't we return EINVAL instead of crashing system? Thanks > > /* Do not send cong updates to IB loopback */ > -- > 2.7.4 > > > -- > Kees Cook > Pixel Security
From: Leon Romanovsky <leon@kernel.org> Date: Mon, 22 Jan 2018 17:10:54 +0200 > On Mon, Jan 22, 2018 at 03:24:15AM -0800, Kees Cook wrote: >> diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c >> index 8557a1cae041..5fbf635d17cb 100644 >> --- a/net/rds/ib_send.c >> +++ b/net/rds/ib_send.c >> @@ -506,7 +506,7 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm, >> int flow_controlled = 0; >> int nr_sig = 0; >> >> - BUG_ON(off % RDS_FRAG_SIZE); >> + BUG_ON(!conn->c_loopback && off % RDS_FRAG_SIZE); >> BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header)); > > To be honest this function full of BUG_ONs and it looks fishy to have them there. > Why don't we return EINVAL instead of crashing system? I completely agree that these assertions should just cause an error-out rather than trigger a BUG().
On 1/22/2018 3:24 AM, Kees Cook wrote: > As described in: https://bugzilla.redhat.com/show_bug.cgi?id=822754 > > Attempting an RDS connection from the IP address of an IPoIB interface > to itself causes a kernel panic due to a BUG_ON() being triggered. > Making the test less strict allows rds-ping to work without crashing > the machine. > > A local unprivileged user could use this flaw to crash the sytem. > Are you able to reproduce this issue on mainline kernel ? IIRC, this sjouldn't happen anymore but if you see it, please let me know. Will try it as well. rds-ping on self loopback device is often tested and used as well for monitoring services in production. > I think this fix was written by Jay Fenlason <fenlason@redhat.com>, > and extracted from the RedHat kernel patches here: > > https://oss.oracle.com/git/gitweb.cgi?p=redpatch.git;a=commitdiff;h=c7b6a0a1d8d636852be130fa15fa8be10d4704e8 > It was part of redhat patched kernel but not carried in shipping Oracle UEK kernels at least afaik. > This fix appears to have been carried by at least RedHat, Oracle, and > Ubuntu for several years. > > CVE-2012-2372 > > Reported-by: Honggang Li <honli@redhat.com> > Cc: stable@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > This is what I get for researching CVE lifetimes... Am not sure if its applicable anymore. Infact the issue with loopback device was due to congestion update and thats been already addressed with commit '18fc25c94: {rds: prevent BUG_ON triggered on congestion update to loopback}' Regards, Santosh
On 1/22/2018 7:47 AM, David Miller wrote: > From: Leon Romanovsky <leon@kernel.org> > Date: Mon, 22 Jan 2018 17:10:54 +0200 > >> On Mon, Jan 22, 2018 at 03:24:15AM -0800, Kees Cook wrote: >>> diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c >>> index 8557a1cae041..5fbf635d17cb 100644 >>> --- a/net/rds/ib_send.c >>> +++ b/net/rds/ib_send.c >>> @@ -506,7 +506,7 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm, >>> int flow_controlled = 0; >>> int nr_sig = 0; >>> >>> - BUG_ON(off % RDS_FRAG_SIZE); >>> + BUG_ON(!conn->c_loopback && off % RDS_FRAG_SIZE); >>> BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header)); >> >> To be honest this function full of BUG_ONs and it looks fishy to have them there. >> Why don't we return EINVAL instead of crashing system? > > I completely agree that these assertions should just cause an error-out > rather than trigger a BUG(). Andy did remove bunch of them but there are still few more left overs. Will have a look at remainder set since most of them were added during early development and remained there. Thanks Dave/Leon. Regards, Santosh
On Tue, Jan 23, 2018 at 4:01 AM, Santosh Shilimkar <santosh.shilimkar@oracle.com> wrote: > On 1/22/2018 3:24 AM, Kees Cook wrote: >> >> As described in: https://bugzilla.redhat.com/show_bug.cgi?id=822754 >> >> Attempting an RDS connection from the IP address of an IPoIB interface >> to itself causes a kernel panic due to a BUG_ON() being triggered. >> Making the test less strict allows rds-ping to work without crashing >> the machine. >> >> A local unprivileged user could use this flaw to crash the sytem. >> > Are you able to reproduce this issue on mainline kernel ? > IIRC, this sjouldn't happen anymore but if you see it, please > let me know. Will try it as well. rds-ping on self > loopback device is often tested and used as well for > monitoring services in production. I don't have an RDS test setup, no. But it sounds like kernels without this patch aren't seeing the problem. > Am not sure if its applicable anymore. Infact the issue with > loopback device was due to congestion update and thats been > already addressed with commit '18fc25c94: {rds: prevent BUG_ON > triggered on congestion update to loopback}' That looks very much like it was fixed there. Thanks! -Kees
On 1/22/2018 2:17 PM, Kees Cook wrote: > On Tue, Jan 23, 2018 at 4:01 AM, Santosh Shilimkar > <santosh.shilimkar@oracle.com> wrote: >> On 1/22/2018 3:24 AM, Kees Cook wrote: >>> >>> As described in: https://bugzilla.redhat.com/show_bug.cgi?id=822754 >>> >>> Attempting an RDS connection from the IP address of an IPoIB interface >>> to itself causes a kernel panic due to a BUG_ON() being triggered. >>> Making the test less strict allows rds-ping to work without crashing >>> the machine. >>> >>> A local unprivileged user could use this flaw to crash the sytem. >>> >> Are you able to reproduce this issue on mainline kernel ? >> IIRC, this sjouldn't happen anymore but if you see it, please >> let me know. Will try it as well. rds-ping on self >> loopback device is often tested and used as well for >> monitoring services in production. > > I don't have an RDS test setup, no. But it sounds like kernels without > this patch aren't seeing the problem. > Yep. Thats what I thought and hence asked. >> Am not sure if its applicable anymore. Infact the issue with >> loopback device was due to congestion update and thats been >> already addressed with commit '18fc25c94: {rds: prevent BUG_ON >> triggered on congestion update to loopback}' > > That looks very much like it was fixed there. Thanks! > Yeah. Thanks Kees !! Regards, Santosh
diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c index 8557a1cae041..5fbf635d17cb 100644 --- a/net/rds/ib_send.c +++ b/net/rds/ib_send.c @@ -506,7 +506,7 @@ int rds_ib_xmit(struct rds_connection *conn, struct rds_message *rm, int flow_controlled = 0; int nr_sig = 0; - BUG_ON(off % RDS_FRAG_SIZE); + BUG_ON(!conn->c_loopback && off % RDS_FRAG_SIZE); BUG_ON(hdr_off != 0 && hdr_off != sizeof(struct rds_header)); /* Do not send cong updates to IB loopback */
As described in: https://bugzilla.redhat.com/show_bug.cgi?id=822754 Attempting an RDS connection from the IP address of an IPoIB interface to itself causes a kernel panic due to a BUG_ON() being triggered. Making the test less strict allows rds-ping to work without crashing the machine. A local unprivileged user could use this flaw to crash the sytem. I think this fix was written by Jay Fenlason <fenlason@redhat.com>, and extracted from the RedHat kernel patches here: https://oss.oracle.com/git/gitweb.cgi?p=redpatch.git;a=commitdiff;h=c7b6a0a1d8d636852be130fa15fa8be10d4704e8 This fix appears to have been carried by at least RedHat, Oracle, and Ubuntu for several years. CVE-2012-2372 Reported-by: Honggang Li <honli@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- This is what I get for researching CVE lifetimes... --- net/rds/ib_send.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)