Date:   Tue, 11 Jan 2022 14:27:18 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Sakari Ailus <sakari.ailus@linux.intel.com>
Cc:     Nick Desaulniers <ndesaulniers@google.com>,
        linux-media@vger.kernel.org, hverkuil@xs4all.nl
Subject: Re: [PATCH 1/1] v4l: Avoid unaligned access warnings when printing
 4cc modifiers
Message-ID: <Yd13pim2nuelGjRq@smile.fi.intel.com>
References: <20220110224656.266536-1-sakari.ailus@linux.intel.com>
 <CAKwvOdnfX1DzgkzCPY7N4LiJDJTGxsKNQbRMmmkt7o5z5O-W9w@mail.gmail.com>
 <Yd1gNZR4rr36ivZV@paasikivi.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Yd1gNZR4rr36ivZV@paasikivi.fi.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Tue, Jan 11, 2022 at 12:47:17PM +0200, Sakari Ailus wrote:
> 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@linux.intel.com> 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.

packed means two things and developers often forgot about the second one:
- the gaps between members in the data structures are removed
- the instance of the data object may be on unaligned address

Here is the second one which results in the warning.

That's why my patch against vsprintf as I explained in that thread.

-- 
With Best Regards,
Andy Shevchenko