| Message ID | 20210412185902.27755-1-tseewald@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usbip: Fix incorrect double assignment to udc->ud.tcp_rx | expand |
On Mon, Apr 12, 2021 at 01:25:20PM -0600, Shuah Khan wrote: > On 4/12/21 1:06 PM, Tom Seewald wrote: > > On 4/12/21 1:59 PM, Tom Seewald wrote: > > > > > commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. > > > > > > Currently udc->ud.tcp_rx is being assigned twice, the second assignment > > > is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. > > > > > > Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") > > > Acked-by: Shuah Khan <skhan@linuxfoundation.org> > > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > > Cc: stable <stable@vger.kernel.org> > > > Addresses-Coverity: ("Unused value") > > > Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > > --- > > > drivers/usb/usbip/vudc_sysfs.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c > > > index f44d98eeb36a..51cc5258b63e 100644 > > > --- a/drivers/usb/usbip/vudc_sysfs.c > > > +++ b/drivers/usb/usbip/vudc_sysfs.c > > > @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, > > > udc->ud.tcp_socket = socket; > > > udc->ud.tcp_rx = tcp_rx; > > > - udc->ud.tcp_rx = tcp_tx; > > > + udc->ud.tcp_tx = tcp_tx; > > > udc->ud.status = SDEV_ST_USED; > > > spin_unlock_irq(&udc->ud.lock); > > I sent this because I believe this patch needs to be backported to the > > 4.9.y and 4.14.y stable trees. > > > > Tom, > > Correct. This needs proting to 4.14 and 4.9. However, you have to also > backport the patch it fixes to 4.14 and 4.9 > > 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") > > You can combine the two patches when you backport to 4.14 and 4.9 and > add both upstream commits in the change log. Please do not ever combine patches for stable submissions, we want to keep things as close as to what happened in Linus's tree as possible as we track commit ids and putting 2 together is pretty impossible to manage over time. thanks, greg k-h
On 4/13/21 1:30 AM, Greg Kroah-Hartman wrote: > On Mon, Apr 12, 2021 at 01:25:20PM -0600, Shuah Khan wrote: >> On 4/12/21 1:06 PM, Tom Seewald wrote: >>> On 4/12/21 1:59 PM, Tom Seewald wrote: >>> >>>> commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. >>>> >>>> Currently udc->ud.tcp_rx is being assigned twice, the second assignment >>>> is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. >>>> >>>> Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") >>>> Acked-by: Shuah Khan <skhan@linuxfoundation.org> >>>> Signed-off-by: Colin Ian King <colin.king@canonical.com> >>>> Cc: stable <stable@vger.kernel.org> >>>> Addresses-Coverity: ("Unused value") >>>> Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com >>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>>> Signed-off-by: Tom Seewald <tseewald@gmail.com> >>>> --- >>>> drivers/usb/usbip/vudc_sysfs.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c >>>> index f44d98eeb36a..51cc5258b63e 100644 >>>> --- a/drivers/usb/usbip/vudc_sysfs.c >>>> +++ b/drivers/usb/usbip/vudc_sysfs.c >>>> @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, >>>> udc->ud.tcp_socket = socket; >>>> udc->ud.tcp_rx = tcp_rx; >>>> - udc->ud.tcp_rx = tcp_tx; >>>> + udc->ud.tcp_tx = tcp_tx; >>>> udc->ud.status = SDEV_ST_USED; >>>> spin_unlock_irq(&udc->ud.lock); >>> I sent this because I believe this patch needs to be backported to the >>> 4.9.y and 4.14.y stable trees. >>> >> >> Tom, >> >> Correct. This needs proting to 4.14 and 4.9. However, you have to also >> backport the patch it fixes to 4.14 and 4.9 >> >> 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") >> >> You can combine the two patches when you backport to 4.14 and 4.9 and >> add both upstream commits in the change log. > > Please do not ever combine patches for stable submissions, we want to > keep things as close as to what happened in Linus's tree as possible as > we track commit ids and putting 2 together is pretty impossible to > manage over time. > Good point. Thank you. thanks, -- Shuah
On 4/12/21 12:59 PM, Tom Seewald wrote: > commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. > > Currently udc->ud.tcp_rx is being assigned twice, the second assignment > is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. > > Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") > Acked-by: Shuah Khan <skhan@linuxfoundation.org> > Signed-off-by: Colin Ian King <colin.king@canonical.com> > Cc: stable <stable@vger.kernel.org> > Addresses-Coverity: ("Unused value") > Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Tom Seewald <tseewald@gmail.com> > --- > drivers/usb/usbip/vudc_sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c > index f44d98eeb36a..51cc5258b63e 100644 > --- a/drivers/usb/usbip/vudc_sysfs.c > +++ b/drivers/usb/usbip/vudc_sysfs.c > @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, > > udc->ud.tcp_socket = socket; > udc->ud.tcp_rx = tcp_rx; > - udc->ud.tcp_rx = tcp_tx; > + udc->ud.tcp_tx = tcp_tx; > udc->ud.status = SDEV_ST_USED; > > spin_unlock_irq(&udc->ud.lock); > Greg, Please pick this up for 4.9 and 4.14 thanks, -- Shuah
On 4/16/21 9:32 AM, Shuah Khan wrote: > On 4/12/21 12:59 PM, Tom Seewald wrote: >> commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. >> >> Currently udc->ud.tcp_rx is being assigned twice, the second assignment >> is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. >> >> Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading >> to gpf") >> Acked-by: Shuah Khan <skhan@linuxfoundation.org> >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> Cc: stable <stable@vger.kernel.org> >> Addresses-Coverity: ("Unused value") >> Link: >> https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com >> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Signed-off-by: Tom Seewald <tseewald@gmail.com> >> --- >> drivers/usb/usbip/vudc_sysfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/usbip/vudc_sysfs.c >> b/drivers/usb/usbip/vudc_sysfs.c >> index f44d98eeb36a..51cc5258b63e 100644 >> --- a/drivers/usb/usbip/vudc_sysfs.c >> +++ b/drivers/usb/usbip/vudc_sysfs.c >> @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, >> udc->ud.tcp_socket = socket; >> udc->ud.tcp_rx = tcp_rx; >> - udc->ud.tcp_rx = tcp_tx; >> + udc->ud.tcp_tx = tcp_tx; >> udc->ud.status = SDEV_ST_USED; >> spin_unlock_irq(&udc->ud.lock); >> > > Greg, > > Please pick this up for 4.9 and 4.14 > Forgot Acked-by :( Acked-by: Shuah Khan <skhan@linuxfoundation.org> thanks, -- Shuah
On Fri, Apr 16, 2021 at 09:32:35AM -0600, Shuah Khan wrote: > On 4/12/21 12:59 PM, Tom Seewald wrote: > > commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. > > > > Currently udc->ud.tcp_rx is being assigned twice, the second assignment > > is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. > > > > Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") > > Acked-by: Shuah Khan <skhan@linuxfoundation.org> > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > Cc: stable <stable@vger.kernel.org> > > Addresses-Coverity: ("Unused value") > > Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > --- > > drivers/usb/usbip/vudc_sysfs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c > > index f44d98eeb36a..51cc5258b63e 100644 > > --- a/drivers/usb/usbip/vudc_sysfs.c > > +++ b/drivers/usb/usbip/vudc_sysfs.c > > @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, > > udc->ud.tcp_socket = socket; > > udc->ud.tcp_rx = tcp_rx; > > - udc->ud.tcp_rx = tcp_tx; > > + udc->ud.tcp_tx = tcp_tx; > > udc->ud.status = SDEV_ST_USED; > > spin_unlock_irq(&udc->ud.lock); > > > > Greg, > > Please pick this up for 4.9 and 4.14 Why? The commit it says it fixes, 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf"), is not in any kernel older than 4.19.y at the moment. Should that commit also be backported? confused, greg k-h
On 4/19/21 6:25 AM, Greg Kroah-Hartman wrote: > On Fri, Apr 16, 2021 at 09:32:35AM -0600, Shuah Khan wrote: >> On 4/12/21 12:59 PM, Tom Seewald wrote: >>> commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. >>> >>> Currently udc->ud.tcp_rx is being assigned twice, the second assignment >>> is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. >>> >>> Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") >>> Acked-by: Shuah Khan <skhan@linuxfoundation.org> >>> Signed-off-by: Colin Ian King <colin.king@canonical.com> >>> Cc: stable <stable@vger.kernel.org> >>> Addresses-Coverity: ("Unused value") >>> Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Signed-off-by: Tom Seewald <tseewald@gmail.com> >>> --- >>> drivers/usb/usbip/vudc_sysfs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c >>> index f44d98eeb36a..51cc5258b63e 100644 >>> --- a/drivers/usb/usbip/vudc_sysfs.c >>> +++ b/drivers/usb/usbip/vudc_sysfs.c >>> @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, >>> udc->ud.tcp_socket = socket; >>> udc->ud.tcp_rx = tcp_rx; >>> - udc->ud.tcp_rx = tcp_tx; >>> + udc->ud.tcp_tx = tcp_tx; >>> udc->ud.status = SDEV_ST_USED; >>> spin_unlock_irq(&udc->ud.lock); >>> >> >> Greg, >> >> Please pick this up for 4.9 and 4.14 > > Why? The commit it says it fixes, 46613c9dfa96 ("usbip: fix vudc > usbip_sockfd_store races leading to gpf"), is not in any kernel older > than 4.19.y at the moment. > Tom back ported this one a couple of weeks ago to 4.14.y and 4.9.y I see this commit in 4.14 (checked on 4.14.231) e9c1341b4c948c20f030b6b146fa82575e2fc37b I see this commit in 4.9.267 as well. fe9e15a30be666ec8071f325a39fe13e2251b51d This fix can go on top of these commits. thanks, -- Shuah
On Mon, Apr 19, 2021 at 04:06:49PM -0600, Shuah Khan wrote: > On 4/19/21 6:25 AM, Greg Kroah-Hartman wrote: > > On Fri, Apr 16, 2021 at 09:32:35AM -0600, Shuah Khan wrote: > > > On 4/12/21 12:59 PM, Tom Seewald wrote: > > > > commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. > > > > > > > > Currently udc->ud.tcp_rx is being assigned twice, the second assignment > > > > is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. > > > > > > > > Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") > > > > Acked-by: Shuah Khan <skhan@linuxfoundation.org> > > > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > > > Cc: stable <stable@vger.kernel.org> > > > > Addresses-Coverity: ("Unused value") > > > > Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > > > --- > > > > drivers/usb/usbip/vudc_sysfs.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c > > > > index f44d98eeb36a..51cc5258b63e 100644 > > > > --- a/drivers/usb/usbip/vudc_sysfs.c > > > > +++ b/drivers/usb/usbip/vudc_sysfs.c > > > > @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, > > > > udc->ud.tcp_socket = socket; > > > > udc->ud.tcp_rx = tcp_rx; > > > > - udc->ud.tcp_rx = tcp_tx; > > > > + udc->ud.tcp_tx = tcp_tx; > > > > udc->ud.status = SDEV_ST_USED; > > > > spin_unlock_irq(&udc->ud.lock); > > > > > > > > > > Greg, > > > > > > Please pick this up for 4.9 and 4.14 > > > > Why? The commit it says it fixes, 46613c9dfa96 ("usbip: fix vudc > > usbip_sockfd_store races leading to gpf"), is not in any kernel older > > than 4.19.y at the moment. > > > > Tom back ported this one a couple of weeks ago to 4.14.y and 4.9.y > > I see this commit in 4.14 (checked on 4.14.231) > e9c1341b4c948c20f030b6b146fa82575e2fc37b > > > I see this commit in 4.9.267 as well. > > fe9e15a30be666ec8071f325a39fe13e2251b51d > > This fix can go on top of these commits. Now queued up, thanks. greg k-h
diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c index f44d98eeb36a..51cc5258b63e 100644 --- a/drivers/usb/usbip/vudc_sysfs.c +++ b/drivers/usb/usbip/vudc_sysfs.c @@ -187,7 +187,7 @@ static ssize_t store_sockfd(struct device *dev, udc->ud.tcp_socket = socket; udc->ud.tcp_rx = tcp_rx; - udc->ud.tcp_rx = tcp_tx; + udc->ud.tcp_tx = tcp_tx; udc->ud.status = SDEV_ST_USED; spin_unlock_irq(&udc->ud.lock);
commit 9858af27e69247c5d04c3b093190a93ca365f33d upstream. Currently udc->ud.tcp_rx is being assigned twice, the second assignment is incorrect, it should be to udc->ud.tcp_tx instead of rx. Fix this. Fixes: 46613c9dfa96 ("usbip: fix vudc usbip_sockfd_store races leading to gpf") Acked-by: Shuah Khan <skhan@linuxfoundation.org> Signed-off-by: Colin Ian King <colin.king@canonical.com> Cc: stable <stable@vger.kernel.org> Addresses-Coverity: ("Unused value") Link: https://lore.kernel.org/r/20210311104445.7811-1-colin.king@canonical.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Tom Seewald <tseewald@gmail.com> --- drivers/usb/usbip/vudc_sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)