Hi Nick,
On Mon, Jan 10, 2022 at 03:11:18PM -0800, Nick Desaulniers wrote:
> On Mon, Jan 10, 2022 at 2:48 PM Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >
> > Pointers V4L2 pixelformat and dataformat fields in a few packed structs
> > are directly passed to printk family of functions.
>
> I would rephrase the below statement...
>
> > This could result in an
> > unaligned access albeit no such possibility appears to exist at the
> > moment i.e. this clang warning appears to be a false positive.
>
> ...to:
>
> warning: taking address of packed member 'pixelformat' of class or
> structure 'v4l2_pix_format_mplane' may result in an unaligned pointer
> value [-Waddress-of-packed-member]
>
> The warning is correct; because `struct v4l2_pix_format_mplane` is
> __packed, it's members also have __aligned(1). Taking the address of
> such members results in the use of underaligned pointers which is UB
> and may be caught by UBSAN or fault on architectures without unaligned
> loads should the struct instance happen to be allocated without any
> natural alignment.
Wouldn't that be the case only if the __packed attribute resulted in a
different memory layout than not having that attribute?
All these fields are aligned by 4 so I don't see how this could be an
actual problem.
>
> >
> > Address the warning by copying the pixelformat or dataformat value to a
> > local variable first.
> >
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Fixes: e927e1e0f0dd ("v4l: ioctl: Use %p4cc printk modifier to print FourCC codes")
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > ---
> > Hi Andy, Nick,
> >
> > How about this one?
> >
> > I believe it does address the clang warning although I haven't tested it.
>
> LGTM. Thanks Sakari and Andy for pursuing this. Just a minor nit on my
> side about the framing of this warning being a false positive; I don't
> think it is. With that amended,
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Thanks!
--
Kind regards,
Sakari Ailus
