| Message ID | 5fc35a2a983a93a88e44bde0492f37a717578d4d.1749591024.git.echaudro@redhat.com |
|---|---|
| State | Accepted |
| Commit | ca9e67c8019fb2389f15147d6147b6610f2ed9fe |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev,v2] daemon-unix: Handle potential negative values from sysconf(). | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Eelco Chaudron <echaudro@redhat.com> writes: > Coverity reports that daemon_set_new_user() may receive a large > unsigned value from get_sysconf_buffer_size(), due to sysconf() > returning -1 and being cast to size_t. > > Although this would likely lead to an allocation failure and abort, > it's better to handle the error in place. > > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > > --- > v2: added comment to the code on -1 return reasons. > --- LGTM, Acked-by: Aaron Conole <aconole@redhat.com>
diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c index 4fdc6e3c4..6d02aceeb 100644 --- a/lib/daemon-unix.c +++ b/lib/daemon-unix.c @@ -915,8 +915,9 @@ daemon_become_new_user(bool access_datapath, bool access_hardware_ports) static size_t get_sysconf_buffer_size(void) { - size_t bufsize, pwd_bs = 0, grp_bs = 0; const size_t default_bufsize = 1024; + long pwd_bs = 0, grp_bs = 0; + size_t bufsize; errno = 0; if ((pwd_bs = sysconf(_SC_GETPW_R_SIZE_MAX)) == -1) { @@ -924,14 +925,24 @@ get_sysconf_buffer_size(void) VLOG_FATAL("%s: Read initial passwordd struct size " "failed (%s), aborting. ", pidfile, ovs_strerror(errno)); + } else { + /* As per API documentation, sysconf(_SC_GETPW_R_SIZE_MAX) may + * return -1 without setting errno, indicating no recommended + * size. In that case, fall back to the default buffer size. */ + pwd_bs = default_bufsize; } } + errno = 0; if ((grp_bs = sysconf(_SC_GETGR_R_SIZE_MAX)) == -1) { if (errno) { VLOG_FATAL("%s: Read initial group struct size " "failed (%s), aborting. ", pidfile, ovs_strerror(errno)); + } else { + /* See above comment on _SC_GETPW_R_SIZE_MAX, as the same applies + * to _SC_GETGR_R_SIZE_MAX. */ + grp_bs = default_bufsize; } }
Coverity reports that daemon_set_new_user() may receive a large unsigned value from get_sysconf_buffer_size(), due to sysconf() returning -1 and being cast to size_t. Although this would likely lead to an allocation failure and abort, it's better to handle the error in place. Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- v2: added comment to the code on -1 return reasons. --- lib/daemon-unix.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)