| Message ID | 20241004123313.2463701-1-bemenboshra2001@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usb: usbtmc: initialize memory written to device | expand |
On Fri, Oct 04, 2024 at 12:33:13PM +0000, Pimyn@web.codeaurora.org wrote: > Avoid kernel-usb-infoleak by initializing all memory written to device. > The buffer length uses 32bit alignment which might cause some buffer > data to be read without any initialization. > > Reported-by: syzbot+8f282cce71948071c335@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/0000000000006f6622061eb52dba@google.com/T/ > Signed-off-by: Pimyn Girgis <bemenboshra2001@gmail.com> (Google) > --- > drivers/usb/class/usbtmc.c | 3 +++ > 1 file changed, 3 insertions(+) The "From:" line of this commit is obviously not correct and does not match this signed-off-by line. Also, as you are from google, just use your google.com email address please. thanks, greg k-h
On Fri, Oct 04, 2024 at 12:33:13PM +0000, Pimyn@web.codeaurora.org wrote: > Avoid kernel-usb-infoleak by initializing all memory written to device. > The buffer length uses 32bit alignment which might cause some buffer > data to be read without any initialization. > > Reported-by: syzbot+8f282cce71948071c335@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/all/0000000000006f6622061eb52dba@google.com/T/ > Signed-off-by: Pimyn Girgis <bemenboshra2001@gmail.com> (Google) > --- > drivers/usb/class/usbtmc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c > index ffc9c6fdd7e1..d79a08dfb54d 100644 > --- a/drivers/usb/class/usbtmc.c > +++ b/drivers/usb/class/usbtmc.c > @@ -21,6 +21,7 @@ > #include <linux/usb.h> > #include <linux/compat.h> > #include <linux/usb/tmc.h> > +#include <linux/string.h> > > /* Increment API VERSION when changing tmc.h with new flags or ioctls > * or when changing a significant behavior of the driver. > @@ -1169,6 +1170,8 @@ static ssize_t usbtmc_generic_write(struct usbtmc_file_data *file_data, > * (size + 3 & ~3) rounds up and simplifies user code > */ > aligned = (this_part + 3) & ~3; > + /* Initialize the remaining part of the buffer */ > + memzero_explicit(buffer + this_part, aligned - this_part); Why "explicit"? Also, what about commit 625fa77151f0 ("USB: usbtmc: prevent kernel-usb-infoleak"), doesn't that solve the issue here? thanks, greg k-h
On Fri, 4 Oct 2024 at 14:58, Greg KH <gregkh@linuxfoundation.org> wrote: > Why "explicit"? Thank you. I think we could use memzero here instead. > Also, what about commit 625fa77151f0 ("USB: usbtmc: prevent > kernel-usb-infoleak"), doesn't that solve the issue here? You're right. This commit is not in 6.11, so I missed it.
On Fri, Oct 4, 2024 at 2:57 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Fri, Oct 04, 2024 at 12:33:13PM +0000, Pimyn@web.codeaurora.org wrote: Just curious, what's up with this @web.codeaurora.org address? It wasn't in the original patch, where did it come from? :) > > Avoid kernel-usb-infoleak by initializing all memory written to device. > > The buffer length uses 32bit alignment which might cause some buffer > > data to be read without any initialization. > > > > Reported-by: syzbot+8f282cce71948071c335@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/all/0000000000006f6622061eb52dba@google.com/T/ > > Signed-off-by: Pimyn Girgis <bemenboshra2001@gmail.com> (Google) > > --- > > drivers/usb/class/usbtmc.c | 3 +++ > > 1 file changed, 3 insertions(+) > > The "From:" line of this commit is obviously not correct and does not > match this signed-off-by line. Sorry, does it mean the "(Google)" part should be present in the From: line as well? > Also, as you are from google, just use your google.com email address > please. Unfortunately Pimyn cannot send emails outside the @google.com domain.
On Fri, Oct 04, 2024 at 03:41:22PM +0200, Alexander Potapenko wrote: > On Fri, Oct 4, 2024 at 2:57 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > On Fri, Oct 04, 2024 at 12:33:13PM +0000, Pimyn@web.codeaurora.org wrote: > > Just curious, what's up with this @web.codeaurora.org address? It > wasn't in the original patch, where did it come from? :) You tell me, look at the original patch, it's in the email... > > > Avoid kernel-usb-infoleak by initializing all memory written to device. > > > The buffer length uses 32bit alignment which might cause some buffer > > > data to be read without any initialization. > > > > > > Reported-by: syzbot+8f282cce71948071c335@syzkaller.appspotmail.com > > > Closes: https://lore.kernel.org/all/0000000000006f6622061eb52dba@google.com/T/ > > > Signed-off-by: Pimyn Girgis <bemenboshra2001@gmail.com> (Google) > > > --- > > > drivers/usb/class/usbtmc.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > The "From:" line of this commit is obviously not correct and does not > > match this signed-off-by line. > > Sorry, does it mean the "(Google)" part should be present in the From: > line as well? A valid email address should be in the "From:" line. > > Also, as you are from google, just use your google.com email address > > please. > > Unfortunately Pimyn cannot send emails outside the @google.com domain. Google has well-known facilities and abilities to handle this, please use them and do not hide behind gmail accounts. thanks, greg k-h
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c index ffc9c6fdd7e1..d79a08dfb54d 100644 --- a/drivers/usb/class/usbtmc.c +++ b/drivers/usb/class/usbtmc.c @@ -21,6 +21,7 @@ #include <linux/usb.h> #include <linux/compat.h> #include <linux/usb/tmc.h> +#include <linux/string.h> /* Increment API VERSION when changing tmc.h with new flags or ioctls * or when changing a significant behavior of the driver. @@ -1169,6 +1170,8 @@ static ssize_t usbtmc_generic_write(struct usbtmc_file_data *file_data, * (size + 3 & ~3) rounds up and simplifies user code */ aligned = (this_part + 3) & ~3; + /* Initialize the remaining part of the buffer */ + memzero_explicit(buffer + this_part, aligned - this_part); dev_dbg(dev, "write(size:%u align:%u done:%u)\n", (unsigned int)this_part, (unsigned int)aligned,
Avoid kernel-usb-infoleak by initializing all memory written to device. The buffer length uses 32bit alignment which might cause some buffer data to be read without any initialization. Reported-by: syzbot+8f282cce71948071c335@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/0000000000006f6622061eb52dba@google.com/T/ Signed-off-by: Pimyn Girgis <bemenboshra2001@gmail.com> (Google) --- drivers/usb/class/usbtmc.c | 3 +++ 1 file changed, 3 insertions(+)