| Message ID | 20220512112037.103142-1-o-takashi@sakamocchi.jp |
|---|---|
| State | Accepted |
| Commit | dda8ad0aa8af937feb5113952fb7886c74315010 |
| Headers | show |
| Series | firewire: cdev: fix potential leak of kernel stack due to uninitialized value | expand |
Hi Iwai-san, I have a moderate request to you for the patch which fixes an issue included in v5.19-rc1. If it's applicable and I can borrow your help again, I'd like you to send the patch to mainline via your tree. If possible, it's preferable to apply additional three patches I respined[1], but it could be optional since not so critical. [1] https://lore.kernel.org/alsa-devel/20220512111756.103008-1-o-takashi@sakamocchi.jp/ On Thu, May 12, 2022 at 08:20:37PM +0900, Takashi Sakamoto wrote: > Recent change brings potential leak of value on kernel stack to userspace > due to uninitialized value. > > This commit fixes the bug. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Fixes: baa914cd81f ("firewire: add kernel API to access CYCLE_TIME register") > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> > --- > drivers/firewire/core-cdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c > index 8e9670036e5c..032ee56c34a3 100644 > --- a/drivers/firewire/core-cdev.c > +++ b/drivers/firewire/core-cdev.c > @@ -1211,7 +1211,7 @@ static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg) > struct fw_cdev_get_cycle_timer2 *a = &arg->get_cycle_timer2; > struct fw_card *card = client->device->card; > struct timespec64 ts = {0, 0}; > - u32 cycle_time; > + u32 cycle_time = 0; > int ret = 0; > > local_irq_disable(); > -- > 2.34.1 Thanks Takashi Sakamoto
On Tue, 14 Jun 2022 14:30:36 +0200, Takashi Sakamoto wrote: > > Hi Iwai-san, > > I have a moderate request to you for the patch which fixes an issue > included in v5.19-rc1. If it's applicable and I can borrow your help > again, I'd like you to send the patch to mainline via your tree. Do you have the lore URL I can get a patch from? > If possible, it's preferable to apply additional three patches I > respined[1], but it could be optional since not so critical. > > [1] https://lore.kernel.org/alsa-devel/20220512111756.103008-1-o-takashi@sakamocchi.jp/ I can merge those, but now looking at the patches, I'm afraid that the patch 2 ("firewire: use struct_size over open coded arithmetic") is wrong; struct_size() takes the number of elements, and the element type is u32, hence you're allocating 4 times large data with that patch. thanks, Takashi
On Tue, Jun 14, 2022 at 03:07:46PM +0200, Takashi Iwai wrote: > On Tue, 14 Jun 2022 14:30:36 +0200, > Takashi Sakamoto wrote: > > > > Hi Iwai-san, > > > > I have a moderate request to you for the patch which fixes an issue > > included in v5.19-rc1. If it's applicable and I can borrow your help > > again, I'd like you to send the patch to mainline via your tree. > > Do you have the lore URL I can get a patch from? Here it is: https://lore.kernel.org/alsa-devel/20220512112037.103142-1-o-takashi@sakamocchi.jp/ > > If possible, it's preferable to apply additional three patches I > > respined[1], but it could be optional since not so critical. > > > > [1] https://lore.kernel.org/alsa-devel/20220512111756.103008-1-o-takashi@sakamocchi.jp/ > > I can merge those, but now looking at the patches, I'm afraid that the > patch 2 ("firewire: use struct_size over open coded arithmetic") is > wrong; struct_size() takes the number of elements, and the element > type is u32, hence you're allocating 4 times large data with that > patch. Indeed, I overlooked it. The length should be quadlet count instead of byte count in the case. I'll post revised patches later. Thanks for your review. Regards Takashi Sakamoto
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 8e9670036e5c..032ee56c34a3 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1211,7 +1211,7 @@ static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg) struct fw_cdev_get_cycle_timer2 *a = &arg->get_cycle_timer2; struct fw_card *card = client->device->card; struct timespec64 ts = {0, 0}; - u32 cycle_time; + u32 cycle_time = 0; int ret = 0; local_irq_disable();
Recent change brings potential leak of value on kernel stack to userspace due to uninitialized value. This commit fixes the bug. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: baa914cd81f ("firewire: add kernel API to access CYCLE_TIME register") Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- drivers/firewire/core-cdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)