Re: [net-next v2 3/3] selftests/net: Test PACKET_AUXDATA

[Joe Damato]

On Sat, Apr 04, 2026 at 11:30:09PM -0400, Willem de Bruijn wrote:
> Willem de Bruijn wrote:
> > Joe Damato wrote:

[...]

> > > -static void do_rx(int fd, int expected_len, char *expected)
> > > +static void check_aux_data(struct cmsghdr *cmsg, int expected_len)
> > >  {
> > > +	struct tpacket_auxdata *adata;
> > > +
> > > +	if (!cmsg)
> > > +		error(1, 0, "auxdata null");
> > > +
> > > +	if (cmsg->cmsg_level != SOL_PACKET)
> > > +		error(1, 0, "cmsg_level != SOL_PACKET");
> > > +
> > > +	if (cmsg->cmsg_type != PACKET_AUXDATA)
> > > +		error(1, 0, "cmsg_type != PACKET_AUXDATA");
> > > +
> > > +	adata = (struct tpacket_auxdata *)CMSG_DATA(cmsg);
> > 
> > Sashiko had another interesting observation that this access may be
> > unaligned, as cmsg_buf[1024] has 1-byte alignment.
> > 
> > That is not new in this patch. Indeed most tests in this dir just
> > deference msg_control as struct cmsghdr * and CMSG_DATA as whatever
> > domain specific type.
> > 
> > The man page also warns about this and suggests using memcpy to
> > access CMSG_DATA. Not sure why it does not warn about the other
> > cmsg_.. fields.
> 
> The commit that introduced that has more context
> 
> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/man/man3/cmsg.3?id=36d25246b4333513fefdbec7f78f29d193cf5d9a
> 
> It points out 32-bit platforms where cmsghdr is 12 bytes.
> 
> At least one example given, ptpd, uses a union to ensure alignment
> 
> 	union {
> 		struct cmsghdr cm;
> 		char	control[256];
> 	}     cmsg_un;
> 
> But at least one other example, ssmping, does not. So I think not even
> the 4B (on 32-bit archs) of cmsghdr fields can be depended on.

I'm fine with adding an "__attribute__((aligned(8)))" to be safe, but FWIW, I
think the key point from the commit message linked above is:
 
  shows access to int payload via *(int *)CMSG_DATA(cmsg) (of course
  int is safe because its alignment is <= header alignment, but this
  is not mentioned).

struct tpacket_auxdata only has u32 and u16, so 4 byte alignment seems fine
and I don't think the issue applies in this particular case. But, as I said
above, I am fine with adding the attribute to be defensive.