[apparmor] [PATCH 02/12] libapparmor: Move over the lib functions needed by cache loading code

[Tyler Hicks]

On 2015-02-13 17:46:26, Seth Arnold wrote:
> On Wed, Dec 10, 2014 at 04:12:23PM -0600, Tyler Hicks wrote:
> > The function names must be prepended with "_aa_" since they're going to
> > be exported from libapparmor. The code bases using the _aa_autofree(),
> > _aa_autoclose(), and _aa_autofclose() will need to internally alias
> > those functions to the previously used autofree, autoclose, and
> > autofclose names.
> > 
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> 
> Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks!

> 
> I think I found an error in the _aa_dirat_for_each() return value:
> 
> > +
> > +/**
> > + * dirat_for_each: iterate over a directory calling cb for each entry
> 
> Should be _aa_dirat_for_each

Fixed in my local branch. It'll be in the eventual v2 that I send out.

> 
> > + * @dir: already opened directory (MAY BE NULL)
> > + * @name: name of the directory (MAY BE NULL)
> > + * @data: data pointer to pass to the callback fn (MAY BE NULL)
> > + * @cb: the callback to pass entry too (NOT NULL)
> > + *
> > + * Iterate over the entries in a directory calling cb for each entry.
> > + * The directory to iterate is determined by a combination of @dir and
> > + * @name.
> > + *
> > + * IF @name is a relative path it is determine relative to at @dir if it
> > + * is specified, else it the lookup is done relative to the current
> > + * working directory.
> > + *
> > + * If @name is not specified then @dir is used as the directory to iterate
> > + * over.
> > + *
> > + * It is an error if both @name and @dir are null
> > + *
> > + * The cb function is called with the DIR in use and the name of the
> > + * file in that directory.  If the file is to be opened it should
> > + * use the openat, fstatat, and related fns.
> > + *
> > + * Returns: 0 on success, else -1 and errno is set to the error code
> 
> Note -1 and errno set on failure...
> 
> > + */
> > +int _aa_dirat_for_each(DIR *dir, const char *name, void *data,
> > +		       int (* cb)(DIR *, const char *, struct stat *, void *))
> > +{
> 
> > +     /* .... */
> 
> > +
> > +	for (error = readdir_r(d, dirent, &ent);     /* XXX */
> > +	     error == 0 && ent != NULL;
> > +	     error = readdir_r(d, dirent, &ent)) {   /* XXX */
> > +		struct stat my_stat;
> > +
> > +		if (strcmp(ent->d_name, ".") == 0 ||
> > +		    strcmp(ent->d_name, "..") == 0)
> > +			continue;
> > +
> > +		if (fstatat(dirfd(d), ent->d_name, &my_stat, 0)) {
> > +			goto fail;
> > +		}
> > +
> > +		if (cb(d, ent->d_name, &my_stat, data)) {
> > +			goto fail;
> > +		}
> > +	}
> > +
> > +	if (d != dir)
> > +		closedir(d);
> > +
> > +	return error;
> 
> If either readdir_r() marked with an /* XXX */ fails, they'll return a
> positive number as the return value, which is then returned via "return
> error". That's not the -1 and errno that the comment suggested.

Nice catch!

I think this is something that should be fixed outside of this patch
set so I've sent a fix to the list, independent of this thread:

  https://lists.ubuntu.com/archives/apparmor/2015-February/007229.html

Tyler
-------------- 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/20150225/2f2ea828/attachment.pgp>