| Message ID | 20220615121505.61412-1-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| Headers | show |
| Series | firewire: fix minor issues | expand |
Hi, I realized that the second patch still includes a bug that shorter buffer is allocated for block request than received length since the computation is aligned to 4 without care of remainder. Actually in the case of block request, the length is not necessarily multiples of 4 and the packet payload has enough size of field with padding to be aligned to 4, according to 1394 OHCI specification. In the implementation of firewire-core driver, the field is copied without the padding. Please abandon them. I'm sorry to trouble you. On Wed, Jun 15, 2022 at 09:15:02PM +0900, Takashi Sakamoto wrote: > This second version of patchset is revised version of previous one[1] to > fix mistake of macro usage pointed out by reviewer[2]. > > As I note, they are not so urgent changes, thus I don't mind postponing > until next merge window. > > [1] https://lore.kernel.org/alsa-devel/20220512111756.103008-1-o-takashi@sakamocchi.jp/ > [2] https://lore.kernel.org/alsa-devel/87o7yvpf4t.wl-tiwai@suse.de/ > > Jiapeng Chong (1): > firewire: convert sysfs sprintf/snprintf family to sysfs_emit > > Lv Ruyi (1): > firewire: Fix using uninitialized value > > Minghao Chi (CGEL ZTE) (1): > firewire: use struct_size over open coded arithmetic > > drivers/firewire/core-device.c | 6 ++---- > drivers/firewire/core-transaction.c | 3 ++- > 2 files changed, 4 insertions(+), 5 deletions(-) > > -- > 2.34.1 Reards Takashi Sakamoto
On Thu, 16 Jun 2022 02:21:42 +0200, Takashi Sakamoto wrote: > > Hi, > > I realized that the second patch still includes a bug that shorter > buffer is allocated for block request than received length since the > computation is aligned to 4 without care of remainder. > > Actually in the case of block request, the length is not necessarily > multiples of 4 and the packet payload has enough size of field with > padding to be aligned to 4, according to 1394 OHCI specification. In the > implementation of firewire-core driver, the field is copied without > the padding. > > Please abandon them. I'm sorry to trouble you. So this implies that the type declaration of data[] rather looks wrong? Takashi
Hi, On Fri, Jun 17, 2022 at 10:42:51AM +0200, Takashi Iwai wrote: > On Thu, 16 Jun 2022 02:21:42 +0200, > Takashi Sakamoto wrote: > > > > Hi, > > > > I realized that the second patch still includes a bug that shorter > > buffer is allocated for block request than received length since the > > computation is aligned to 4 without care of remainder. > > > > Actually in the case of block request, the length is not necessarily > > multiples of 4 and the packet payload has enough size of field with > > padding to be aligned to 4, according to 1394 OHCI specification. In the > > implementation of firewire-core driver, the field is copied without > > the padding. > > > > Please abandon them. I'm sorry to trouble you. > > So this implies that the type declaration of data[] rather looks > wrong? Your great insight. Indeed, I can not find any code to dereference the array for u32 element. In all of cases, the 'struct fw_request.data' is passed losing its pointer type (void *), then copied by the length in byte count. At least, I can not find any warning or error at compiling the driver after replacing the 'u32 []' with 'u8 []'. Even if it were dereferenced, accessing over allocation boundary hardly occurred since typical implementation of slab allocator maintains various sizes of memory objects but multiples of 4. It's possible to declare it with byte array, I think. Thanks Takashi Sakamoto