Alsa-Devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Sakamoto <o-takashi@sakamocchi.jp>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, stefanr@s5r6.in-berlin.de,
	linux1394-devel@lists.sourceforge.net
Subject: Re: [PATCH v2 0/3] firewire: fix minor issues
Date: Sat, 18 Jun 2022 23:28:11 +0900	[thread overview]
Message-ID: <Yq3g+6+x+S0aKv8e@workstation> (raw)
In-Reply-To: <87zgib1y0k.wl-tiwai@suse.de>

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

      reply	other threads:[~2022-06-18 14:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-15 12:15 [PATCH v2 0/3] firewire: fix minor issues Takashi Sakamoto
2022-06-15 12:15 ` [PATCH v2 1/3] firewire: convert sysfs sprintf/snprintf family to sysfs_emit Takashi Sakamoto
2022-06-17  8:44   ` Takashi Iwai
2022-06-15 12:15 ` [PATCH v2 2/3] firewire: use struct_size over open coded arithmetic Takashi Sakamoto
2022-06-15 12:15 ` [PATCH v2 3/3] firewire: Fix using uninitialized value Takashi Sakamoto
2022-06-16  0:21 ` [PATCH v2 0/3] firewire: fix minor issues Takashi Sakamoto
2022-06-17  8:42   ` Takashi Iwai
2022-06-18 14:28     ` Takashi Sakamoto [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yq3g+6+x+S0aKv8e@workstation \
    --to=o-takashi@sakamocchi.jp \
    --cc=alsa-devel@alsa-project.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    --cc=stefanr@s5r6.in-berlin.de \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox