| Message ID | 20241105183936.252530-2-cgoettsche@seltendoof.de (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | [1/2] libselinux: make use of calloc(3) | expand |
On Tue, Nov 5, 2024 at 1:46 PM Christian Göttsche <cgoettsche@seltendoof.de> wrote: > > From: Christian Göttsche <cgzones@googlemail.com> > > openattr() supplies the simplementation for the getcon(3) interface implementation > family. Use a short local buffer instead of descend into memory "Use a short local buffer instead of dynamic memory allocation"? > allocation. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > libselinux/src/procattr.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c > index ddcc7f8d..ee1f48af 100644 > --- a/libselinux/src/procattr.c > +++ b/libselinux/src/procattr.c > @@ -86,32 +86,32 @@ static void init_procattr(void) > static int openattr(pid_t pid, const char *attr, int flags) > { > int fd, rc; > - char *path; > + char path[56]; /* must hold "/proc/self/task/%d/attr/sockcreate" */ Why 56? I understand that sockcreate is the largest attr, but it looks to me like you are giving way more space than is needed for the pid. I am just curious. Maybe I am missing something. Thanks, Jim > pid_t tid; > > if (pid > 0) { > - rc = asprintf(&path, "/proc/%d/attr/%s", pid, attr); > + rc = snprintf(path, sizeof(path), "/proc/%d/attr/%s", pid, attr); > } else if (pid == 0) { > - rc = asprintf(&path, "/proc/thread-self/attr/%s", attr); > - if (rc < 0) > + rc = snprintf(path, sizeof(path), "/proc/thread-self/attr/%s", attr); > + if (rc < 0 || (size_t)rc >= sizeof(path)) { > + errno = EOVERFLOW; > return -1; > + } > fd = open(path, flags | O_CLOEXEC); > if (fd >= 0 || errno != ENOENT) > - goto out; > - free(path); > + return fd; > tid = selinux_gettid(); > - rc = asprintf(&path, "/proc/self/task/%d/attr/%s", tid, attr); > + rc = snprintf(path, sizeof(path), "/proc/self/task/%d/attr/%s", tid, attr); > } else { > errno = EINVAL; > return -1; > } > - if (rc < 0) > + if (rc < 0 || (size_t)rc >= sizeof(path)) { > + errno = EOVERFLOW; > return -1; > + } > > - fd = open(path, flags | O_CLOEXEC); > -out: > - free(path); > - return fd; > + return open(path, flags | O_CLOEXEC); > } > > static int getprocattrcon_raw(char **context, pid_t pid, const char *attr, > -- > 2.45.2 > >
diff --git a/libselinux/src/procattr.c b/libselinux/src/procattr.c index ddcc7f8d..ee1f48af 100644 --- a/libselinux/src/procattr.c +++ b/libselinux/src/procattr.c @@ -86,32 +86,32 @@ static void init_procattr(void) static int openattr(pid_t pid, const char *attr, int flags) { int fd, rc; - char *path; + char path[56]; /* must hold "/proc/self/task/%d/attr/sockcreate" */ pid_t tid; if (pid > 0) { - rc = asprintf(&path, "/proc/%d/attr/%s", pid, attr); + rc = snprintf(path, sizeof(path), "/proc/%d/attr/%s", pid, attr); } else if (pid == 0) { - rc = asprintf(&path, "/proc/thread-self/attr/%s", attr); - if (rc < 0) + rc = snprintf(path, sizeof(path), "/proc/thread-self/attr/%s", attr); + if (rc < 0 || (size_t)rc >= sizeof(path)) { + errno = EOVERFLOW; return -1; + } fd = open(path, flags | O_CLOEXEC); if (fd >= 0 || errno != ENOENT) - goto out; - free(path); + return fd; tid = selinux_gettid(); - rc = asprintf(&path, "/proc/self/task/%d/attr/%s", tid, attr); + rc = snprintf(path, sizeof(path), "/proc/self/task/%d/attr/%s", tid, attr); } else { errno = EINVAL; return -1; } - if (rc < 0) + if (rc < 0 || (size_t)rc >= sizeof(path)) { + errno = EOVERFLOW; return -1; + } - fd = open(path, flags | O_CLOEXEC); -out: - free(path); - return fd; + return open(path, flags | O_CLOEXEC); } static int getprocattrcon_raw(char **context, pid_t pid, const char *attr,