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

[Dwight Engen]

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.


#include <stdio.h>
#include <linux/netlink.h>
#include <linux/genetlink.h>

struct genlmsg {
	struct nlmsghdr nlmsghdr;
	struct genlmsghdr genlmsghdr;
};

int main(int argc, char *argv[])
{
	struct genlmsg *request = NULL;
	struct genlmsghdr *p1,*p2;

	printf("sizeof request:%lu\n", sizeof(*request));
	printf("sizeof nlmsghdr:%lu\n", sizeof(request->nlmsghdr));

	p1 = NLMSG_DATA(&request->nlmsghdr);
	p2 = &request->genlmsghdr;
	printf("p1:%p p2:%p\n", p1, p2);
	return 0;
}