Joe Damato wrote:
> 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).
But that conditional on having a union as showed to make sure that
the structure has the struct cmsghdr alignment.
A bare char[] won't have that.
Anyway, in practice on the platforms we test this hasn't been an issue.
Fine to copy the example from other tests, leaving out the alignment.
I will follow-up with one patch to update all of them at once, once
I'm certain that I did not overlook some other mechanism that
implicitly does guarantee this alignment on all platforms.
> 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.
