[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 0/6] sVirt AppArmor security driver

[ Sending again as my mail Sat, Sep 05, 2009 at 11:00:46AM +0200 didn't
  seem to have reach the list, Daniel ]

On Fri, Sep 04, 2009 at 04:03:50PM -0500, Jamie Strandboge wrote:
> On Fri, 04 Sep 2009, Daniel Veillard wrote:
> > On Fri, Sep 04, 2009 at 12:23:33PM -0500, Jamie Strandboge wrote:
> > > This patch series implements the AppArmor security driver for sVirt.
> > > This implementation was developed for the Ubuntu AppArmorLibvirtProfile
> > > specification[1], but is general enough for any AppArmor deployment
> > > (such as Ubuntu, *SUSE and Mandriva).
> > > 
> > > This patch has seen quite a bit of real world testing in Ubuntu 9.10
> > > (our development release) in our 0.7.0-1ubuntu3 package. I did make a
> > > few small changes after going through HACKING, but mostly I got the
> > > tests going and added documentation.
> > 
> >   Could you triple check that the make syntax-check run clean with
> > your patches applied, because a very quick glimpse shows up a number
> > of malloc() which we don't allow for example. It may be because you
> > didn't enter the files in the SCM but the checks should be done anyway
> > on the new files please.
> > 
> I noticed that syntax-check wouldn't scan the new files unless I did a
> git commit first. I did not run 'syntax-check' after adding the
> documention examples, and just found that examples/apparmor/libvirt-qemu
> had a trailing blank line (I was a bit surprised syntax-check checked
> these files (attached is an updated docs patch)).


> I can demonstrate that the files are being checked by removing the
> '#include <config.h>' line in security_apparmor.c and virt-aa-helper.c
> and syntax-check failing:
> I don't see that 'syntax-check' enforces not using *alloc, but I did
> read in HACKING that *alloc are deprecated, though I had already written
> the code before reading it. I do see other examples of *alloc in the

  I really though all cases of direct malloc or realloc had been
eliminated, apparently it's not the case yet, and syntax-check doesn't
enforce it... we need to do this. I note we also still have a few free()
left around in the code base to cleanup too.

> codebase, and I do properly check the return code of all calls to *alloc.
> If you insist on those calls being replaced, I can do that, but I didn't
> before submitting because I knew the current calls were correct and
> didn't want to introduce bugs into the code.

  yeah, I think it's better to fix them before. In any case we have a
week of bug fix only before the next release in a week, so best is to
fix them now, wait for the actual reviews and then resubmit patches
for inclusion.



Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]