* net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable @ 2019-05-20 12:40 Pavel Machek 2019-05-20 14:00 ` Greg KH 2019-05-20 14:15 ` Gustavo A. R. Silva 0 siblings, 2 replies; 8+ messages in thread From: Pavel Machek @ 2019-05-20 12:40 UTC (permalink / raw) To: stable, kernel list, gustavo, davem, gregkh [-- Attachment #1: Type: text/plain, Size: 714 bytes --] In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix changed that. Bug does not exist in mainline AFAICT. Signed-off-by: Pavel Machek <pavel@denx.de> # for 4.19.y diff --git a/net/atm/lec.c b/net/atm/lec.c index ad4f829193f0..ed279cd912f4 100644 --- a/net/atm/lec.c +++ b/net/atm/lec.c @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) i = arg; if (arg >= MAX_LEC_ITF) return -EINVAL; - i = array_index_nospec(arg, MAX_LEC_ITF); + i = array_index_nospec(i, MAX_LEC_ITF); if (!dev_lec[i]) { int size; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 12:40 net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable Pavel Machek @ 2019-05-20 14:00 ` Greg KH 2019-05-20 14:26 ` Gustavo A. R. Silva 2019-05-20 16:15 ` Pavel Machek 2019-05-20 14:15 ` Gustavo A. R. Silva 1 sibling, 2 replies; 8+ messages in thread From: Greg KH @ 2019-05-20 14:00 UTC (permalink / raw) To: Pavel Machek; +Cc: stable, kernel list, gustavo, davem On Mon, May 20, 2019 at 02:40:14PM +0200, Pavel Machek wrote: > > In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix > changed that. Bug does not exist in mainline AFAICT. > > Signed-off-by: Pavel Machek <pavel@denx.de> > # for 4.19.y > > diff --git a/net/atm/lec.c b/net/atm/lec.c > index ad4f829193f0..ed279cd912f4 100644 > --- a/net/atm/lec.c > +++ b/net/atm/lec.c > @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) > i = arg; > if (arg >= MAX_LEC_ITF) > return -EINVAL; > - i = array_index_nospec(arg, MAX_LEC_ITF); > + i = array_index_nospec(i, MAX_LEC_ITF); > if (!dev_lec[i]) { > int size; > Why is this only for 4.19.y? What is different in Linus's tree that makes this not needed there? thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 14:00 ` Greg KH @ 2019-05-20 14:26 ` Gustavo A. R. Silva 2019-05-20 21:00 ` Pavel Machek 2019-05-20 16:15 ` Pavel Machek 1 sibling, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2019-05-20 14:26 UTC (permalink / raw) To: Greg KH, Pavel Machek; +Cc: stable, kernel list, davem On 5/20/19 9:00 AM, Greg KH wrote: > On Mon, May 20, 2019 at 02:40:14PM +0200, Pavel Machek wrote: >> >> In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix >> changed that. Bug does not exist in mainline AFAICT. >> >> Signed-off-by: Pavel Machek <pavel@denx.de> >> # for 4.19.y >> >> diff --git a/net/atm/lec.c b/net/atm/lec.c >> index ad4f829193f0..ed279cd912f4 100644 >> --- a/net/atm/lec.c >> +++ b/net/atm/lec.c >> @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) >> i = arg; >> if (arg >= MAX_LEC_ITF) >> return -EINVAL; >> - i = array_index_nospec(arg, MAX_LEC_ITF); >> + i = array_index_nospec(i, MAX_LEC_ITF); >> if (!dev_lec[i]) { >> int size; >> > > Why is this only for 4.19.y? What is different in Linus's tree that > makes this not needed there? > The only difference is this clean up: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fdd1a8103a6df50bdeacd8bb04c3f6976cb9ae41 As Dan says, the code works fine, but the *i* value wasn't being used anymore, so that piece of code was a bit confusing. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 14:26 ` Gustavo A. R. Silva @ 2019-05-20 21:00 ` Pavel Machek 0 siblings, 0 replies; 8+ messages in thread From: Pavel Machek @ 2019-05-20 21:00 UTC (permalink / raw) To: Gustavo A. R. Silva; +Cc: Greg KH, Pavel Machek, stable, kernel list, davem [-- Attachment #1: Type: text/plain, Size: 1624 bytes --] On Mon 2019-05-20 09:26:45, Gustavo A. R. Silva wrote: > > > On 5/20/19 9:00 AM, Greg KH wrote: > > On Mon, May 20, 2019 at 02:40:14PM +0200, Pavel Machek wrote: > >> > >> In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix > >> changed that. Bug does not exist in mainline AFAICT. > >> > >> Signed-off-by: Pavel Machek <pavel@denx.de> > >> # for 4.19.y > >> > >> diff --git a/net/atm/lec.c b/net/atm/lec.c > >> index ad4f829193f0..ed279cd912f4 100644 > >> --- a/net/atm/lec.c > >> +++ b/net/atm/lec.c > >> @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) > >> i = arg; > >> if (arg >= MAX_LEC_ITF) > >> return -EINVAL; > >> - i = array_index_nospec(arg, MAX_LEC_ITF); > >> + i = array_index_nospec(i, MAX_LEC_ITF); > >> if (!dev_lec[i]) { > >> int size; > >> > > > > Why is this only for 4.19.y? What is different in Linus's tree that > > makes this not needed there? > > > > The only difference is this clean up: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fdd1a8103a6df50bdeacd8bb04c3f6976cb9ae41 > > As Dan says, the code works fine, but the *i* value wasn't being used > anymore, so that piece of code was a bit confusing. Yep, you are right, code managed to confused me: array_index_nospec really returns 0 if it is out of bounds, (typeof(_i)) (_i & _mask); because _mask is always 0 or ~0. Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 14:00 ` Greg KH 2019-05-20 14:26 ` Gustavo A. R. Silva @ 2019-05-20 16:15 ` Pavel Machek 1 sibling, 0 replies; 8+ messages in thread From: Pavel Machek @ 2019-05-20 16:15 UTC (permalink / raw) To: Greg KH; +Cc: Pavel Machek, stable, kernel list, gustavo, davem [-- Attachment #1: Type: text/plain, Size: 1152 bytes --] On Mon 2019-05-20 16:00:07, Greg KH wrote: > On Mon, May 20, 2019 at 02:40:14PM +0200, Pavel Machek wrote: > > > > In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix > > changed that. Bug does not exist in mainline AFAICT. > > > > Signed-off-by: Pavel Machek <pavel@denx.de> > > # for 4.19.y > > > > diff --git a/net/atm/lec.c b/net/atm/lec.c > > index ad4f829193f0..ed279cd912f4 100644 > > --- a/net/atm/lec.c > > +++ b/net/atm/lec.c > > @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) > > i = arg; > > if (arg >= MAX_LEC_ITF) > > return -EINVAL; > > - i = array_index_nospec(arg, MAX_LEC_ITF); > > + i = array_index_nospec(i, MAX_LEC_ITF); > > if (!dev_lec[i]) { > > int size; > > > > Why is this only for 4.19.y? What is different in Linus's tree that > makes this not needed there? Mainline sanitizes "arg", 4.19 copies "arg" into "i" then sanitizes it. Take a look, it is local to the function. Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 12:40 net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable Pavel Machek 2019-05-20 14:00 ` Greg KH @ 2019-05-20 14:15 ` Gustavo A. R. Silva 2019-05-20 16:15 ` Pavel Machek 1 sibling, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2019-05-20 14:15 UTC (permalink / raw) To: Pavel Machek, stable, kernel list, davem, gregkh On 5/20/19 7:40 AM, Pavel Machek wrote: > > In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix > changed that. Bug does not exist in mainline AFAICT. > NACK array_index_nospec() macro returns zero if *arg* is out of bounds. In any case, if the "bug" does not exist in mainline, what should be done is to port the code that fixed it in mainline to old kernels, not randomly add a "fix" here and there because that means we are breaking backward compatibility. -- Gustavo > Signed-off-by: Pavel Machek <pavel@denx.de> > # for 4.19.y > > diff --git a/net/atm/lec.c b/net/atm/lec.c > index ad4f829193f0..ed279cd912f4 100644 > --- a/net/atm/lec.c > +++ b/net/atm/lec.c > @@ -731,7 +731,7 @@ static int lecd_attach(struct atm_vcc *vcc, int arg) > i = arg; > if (arg >= MAX_LEC_ITF) > return -EINVAL; > - i = array_index_nospec(arg, MAX_LEC_ITF); > + i = array_index_nospec(i, MAX_LEC_ITF); > if (!dev_lec[i]) { > int size; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 14:15 ` Gustavo A. R. Silva @ 2019-05-20 16:15 ` Pavel Machek 2019-05-20 16:54 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Pavel Machek @ 2019-05-20 16:15 UTC (permalink / raw) To: Gustavo A. R. Silva; +Cc: Pavel Machek, stable, kernel list, davem, gregkh [-- Attachment #1: Type: text/plain, Size: 621 bytes --] On Mon 2019-05-20 09:15:50, Gustavo A. R. Silva wrote: > > > On 5/20/19 7:40 AM, Pavel Machek wrote: > > > > In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix > > changed that. Bug does not exist in mainline AFAICT. > > > > NACK > > array_index_nospec() macro returns zero if *arg* is out of bounds. No, it does not. Take a look: #define array_index_nospec(index, size) ... (typeof(_i)) (_i & _mask); }) Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable 2019-05-20 16:15 ` Pavel Machek @ 2019-05-20 16:54 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2019-05-20 16:54 UTC (permalink / raw) To: Pavel Machek; +Cc: stable, kernel list, davem, gregkh On 5/20/19 11:15 AM, Pavel Machek wrote: > On Mon 2019-05-20 09:15:50, Gustavo A. R. Silva wrote: >> >> >> On 5/20/19 7:40 AM, Pavel Machek wrote: >>> >>> In lecd_attach, if arg is < 0, it was treated as 0. Spectre v1 fix >>> changed that. Bug does not exist in mainline AFAICT. >>> >> >> NACK >> >> array_index_nospec() macro returns zero if *arg* is out of bounds. > > No, it does not. Take a look: > > #define array_index_nospec(index, size) > ... > (typeof(_i)) (_i & _mask); > }) > Tell me more about that _mask. Also, don't ignore the backward compatibility; otherwise you would be forking your own kernel. -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-05-20 21:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-20 12:40 net: atm: Spectre v1 fix introduced bug in bcb964012d1b in -stable Pavel Machek 2019-05-20 14:00 ` Greg KH 2019-05-20 14:26 ` Gustavo A. R. Silva 2019-05-20 21:00 ` Pavel Machek 2019-05-20 16:15 ` Pavel Machek 2019-05-20 14:15 ` Gustavo A. R. Silva 2019-05-20 16:15 ` Pavel Machek 2019-05-20 16:54 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).