[lxc-devel] [PATCH 1/4] coverity: don't reference beyond structure

[Dwight Engen]

On Fri, 3 May 2013 13:47:59 -0400
Dwight Engen <dwight.engen at oracle.com> wrote:

> On Fri, 3 May 2013 12:23:29 -0500
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > On Fri, 3 May 2013 11:17:08 -0500
> > > Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> > > 
> > > > can that be correct?  That changes the value of the
> > > > pointer.  NLMSG_DATA adds aligned sizeof(struct nlmsghdr).
> > > 
> > > The current code is relying on the fact that our genlmsg looks
> > > like this:
> > > 
> > > struct genlmsg {
> > > 	struct nlmsghdr nlmsghdr;
> > > 	struct genlmsghdr genlmsghdr;
> > > };
> > > 
> > > so NLMSG_DATA(&request->nlmsghdr) == genlmsghdr (I think sizeof
> > > struct nlmsghdr will make genlmsghdr be aligned anyway?). Right
> > > now lxc doesn't use this code so I cannot test it in lxc, but let
> > > me write a test program that dumps the pointers using the old way
> > > and the new way. We do have a 2*PAGE_SIZE buffer these point to
> > > so I'm sure its not overflowing, so maybe we do just want to
> > > ignore coverity.
> > 
> > Thanks, I'll go ahead and apply the rest and hold off on this one.
> 
> Yeah I agree we should hold off. The following test program shows them
> equivalent, but using NLMSG_DATA may be more appropriate. The
> commented out __attribute__ ((aligned(4))); in genl.h make me
> suspicous this has come up before. Maybe I can look into a way to
> make coverity happy instead.

Just to follow up, I did check what libnl does, and it has nlmsg_data()
that just adds NLMSG_HDRLEN to a nlmsghdr pointer. This is equivalent
to of our use of NLMSG_DATA so I think we should just leave the code as
is and not apply this patch.