[apparmor] [patch 2/5] group network rule bits into their own file

[Steve Beattie]

[Sorry for the delay on the response to this; while looking at the v2
patch, I'm going back to round one to see what issues should have been
addressed.]

On Thu, Aug 07, 2014 at 04:03:35PM -0700, Seth Arnold wrote:
> On Wed, Aug 06, 2014 at 05:32:46AM -0700, john.johansen at canonical.com wrote:
> > Signed-off-by: John Johansen <john.johansen at canonical.com>
> 
> I found a bug; it and other comments inline.
> 

> > +struct aa_network_entry *network_entry(const char *family, const char *type,
> > +				       const char *protocol)
> > +{
> > +	int i;
> > +	struct aa_network_entry *new_entry, *entry = NULL;
> > +
> > +	for (i = 0; network_mappings[i].family_name; i++) {
> > +		if (family) {
> > +			PDEBUG("Checking family %s\n", network_mappings[i].family_name);
> > +			if (strcmp(family, network_mappings[i].family_name) != 0)
> > +				continue;
> > +			PDEBUG("Found family %s\n", family);
> > +		}
> > +		if (type) {
> > +			PDEBUG("Checking type %s\n", network_mappings[i].type_name);
> > +			if (strcmp(type, network_mappings[i].type_name) != 0)
> > +				continue;
> > +			PDEBUG("Found type %s\n", type);
> > +		}
> > +		if (protocol) {
> > +			PDEBUG("Checking protocol type %s\n", network_mappings[i].protocol_name);
> > +			if (strcmp(type, network_mappings[i].protocol_name) != 0)
> > +				continue;
> 
> This strcmp checks type instead of protocol. The PDEBUG ought to be
> changed to "Checking protocol %s\n" while you're here.
> 
[Same issue]
> 
> Ohhhhh. Fixing this bug looks like it might affect existing profiles.

Actually, it won't. I'd had this bug raised by automated tools before,
but if you then grep for all calls to network_entry() in the parser
tree, you see that the protocol argument (the 3rd one) is always passed
as NULL:

  [grep with v2 of the patch applied]

  $ grep network_entry\( -r *
  network.c:struct aa_network_entry *network_entry(const char *family, const char *type,
  network.h:extern struct aa_network_entry *network_entry(const char *family,
  parser_yacc.y:          entry = network_entry($2, NULL, NULL);
  parser_yacc.y:                  entry = network_entry(NULL, $2, NULL);
  parser_yacc.y:          entry = network_entry($2, $3, NULL);

So the protocol test can never be true. I haven't thoroughly examined
the patches following in the v2 set, but I don't see anything that adds
or changes that state.

Now, that may be a bug in its own right.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140815/81b6d85e/attachment.pgp>