[apparmor] [PATCH 2/6] libapparmor: Introduce a single function for reading features files
Tyler Hicks
tyhicks at canonical.com
Thu Apr 2 00:51:44 UTC 2015
On 2015-04-01 17:43:53, Seth Arnold wrote:
> On Thu, Mar 26, 2015 at 04:47:58PM -0500, Tyler Hicks wrote:
> > Two different implementations were in use for reading features files.
> > One for reading a single file and another for reading a single file
> > after walking a directory. This patch creates a single function that is
> > used in both cases.
> >
> > Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
>
> Include the same 'dirfd' complaint from earlier. with the same either
> ignore it because openat() ignores it rationale, or change the name
> because it might collide against dirent.h.
I wonder if the collision is a good thing. It'll make the next person
think twice before mixing directory streams and directory file
descriptors in the same section of code.
Tyler
>
> Acked-by: Seth Arnold <seth.arnold at canonical.com>
>
> Thanks
>
> > ---
> > libraries/libapparmor/src/features.c | 114 +++++++++++++++++++++--------------
> > 1 file changed, 68 insertions(+), 46 deletions(-)
> >
> > diff --git a/libraries/libapparmor/src/features.c b/libraries/libapparmor/src/features.c
> > index e7bf745..f913e91 100644
> > --- a/libraries/libapparmor/src/features.c
> > +++ b/libraries/libapparmor/src/features.c
> > @@ -80,6 +80,69 @@ static int features_snprintf(struct features_struct *fst, const char *fmt, ...)
> > return 0;
> > }
> >
> > +/* load_features_file - opens and reads a file into @buffer and then NUL-terminates @buffer
> > + * @dirfd: a directory file descriptory or AT_FDCWD (see openat(2))
> > + * @path: name of the file
> > + * @buffer: the buffer to read the features file into (will be NUL-terminated on success)
> > + * @size: the size of @buffer
> > + *
> > + * Returns: The number of bytes copied into @buffer on success (not counting
> > + * the NUL-terminator), else -1 and errno is set. Note that @size must be
> > + * larger than the size of the file or -1 will be returned with errno set to
> > + * ENOBUFS indicating that @buffer was not large enough to contain all of the
> > + * file contents.
> > + */
> > +static int load_features_file(int dirfd, const char *path,
> > + char *buffer, size_t size)
> > +{
> > + autoclose int file = -1;
> > + char *pos = buffer;
> > + ssize_t len;
> > +
> > + file = openat(dirfd, path, O_RDONLY);
> > + if (file < 0) {
> > + PDEBUG("Could not open '%s'\n", path);
> > + return -1;
> > + }
> > + PDEBUG("Opened features \"%s\"\n", path);
> > +
> > + if (!size) {
> > + errno = ENOBUFS;
> > + return -1;
> > + }
> > +
> > + /* Save room for a NUL-terminator at the end of @buffer */
> > + size--;
> > +
> > + do {
> > + len = read(file, pos, size);
> > + if (len > 0) {
> > + size -= len;
> > + pos += len;
> > + }
> > + } while (len > 0 && size);
> > +
> > + /**
> > + * Possible error conditions:
> > + * - len == -1: read failed and errno is already set so return -1
> > + * - len > 0: read() never returned 0 (EOF) meaning that @buffer was
> > + * too small to contain all of the file contents so set
> > + * errno to ENOBUFS and return -1
> > + */
> > + if (len) {
> > + if (len > 0)
> > + errno = ENOBUFS;
> > +
> > + PDEBUG("Error reading features file '%s': %m\n", path);
> > + return -1;
> > + }
> > +
> > + /* NUL-terminate @buffer */
> > + *pos = 0;
> > +
> > + return pos - buffer;
> > +}
> > +
> > static int features_dir_cb(int dirfd, const char *name, struct stat *st,
> > void *data)
> > {
> > @@ -93,34 +156,14 @@ static int features_dir_cb(int dirfd, const char *name, struct stat *st,
> > return -1;
> >
> > if (S_ISREG(st->st_mode)) {
> > - autoclose int file = -1;
> > int len;
> > int remaining = fst->size - (fst->pos - fst->buffer);
> >
> > - file = openat(dirfd, name, O_RDONLY);
> > - if (file == -1) {
> > - PDEBUG("Could not open '%s'", name);
> > - return -1;
> > - }
> > - PDEBUG("Opened features \"%s\"\n", name);
> > - if (st->st_size > remaining) {
> > - PDEBUG("Feature buffer full.");
> > - errno = ENOBUFS;
> > + len = load_features_file(dirfd, name, fst->pos, remaining);
> > + if (len < 0)
> > return -1;
> > - }
> >
> > - do {
> > - len = read(file, fst->pos, remaining);
> > - if (len > 0) {
> > - remaining -= len;
> > - fst->pos += len;
> > - *fst->pos = 0;
> > - }
> > - } while (len > 0);
> > - if (len < 0) {
> > - PDEBUG("Error reading feature file '%s'\n", name);
> > - return -1;
> > - }
> > + fst->pos += len;
> > } else if (S_ISDIR(st->st_mode)) {
> > if (_aa_dirat_for_each(dirfd, name, fst, features_dir_cb))
> > return -1;
> > @@ -145,27 +188,6 @@ static int handle_features_dir(const char *filename, char *buffer, int size,
> > return 0;
> > }
> >
> > -static int load_features_file(const char *name, char *buffer, size_t size)
> > -{
> > - autofclose FILE *f = NULL;
> > - size_t end;
> > -
> > - f = fopen(name, "r");
> > - if (!f)
> > - return -1;
> > -
> > - errno = 0;
> > - end = fread(buffer, 1, size - 1, f);
> > - if (ferror(f)) {
> > - if (!errno)
> > - errno = EIO;
> > - return -1;
> > - }
> > - buffer[end] = 0;
> > -
> > - return 0;
> > -}
> > -
> > static bool islbrace(int c)
> > {
> > return c == '{';
> > @@ -362,8 +384,8 @@ int aa_features_new(aa_features **features, const char *path)
> >
> > retval = S_ISDIR(stat_file.st_mode) ?
> > handle_features_dir(path, f->string, STRING_SIZE, f->string) :
> > - load_features_file(path, f->string, STRING_SIZE);
> > - if (retval) {
> > + load_features_file(AT_FDCWD, path, f->string, STRING_SIZE);
> > + if (retval == -1) {
> > int save = errno;
> >
> > aa_features_unref(f);
> > --
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- 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/20150401/f032ee75/attachment-0001.pgp>
More information about the AppArmor
mailing list