| Message ID | 20220512112037.103142-1-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | New, archived |
| 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
On Wed, 15 Jun 2022 03:27:33 +0200, Takashi Sakamoto wrote: > > 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/ Thanks, now applied. The Fixes tag had a too short hash (only 11 letters), so I corrected locally. thanks, Takashi
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(-)