Eelco Chaudron via dev <ovs-dev@openvswitch.org> 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.
That's weird that it's complaining about the upcast. I might be
misremembering the casting rules, though. I did check the latest
c-standard (c22) and it did have this to say in 6.3.1.8 about
conversions in comparisons::
Otherwise, if the operand that has unsigned integer type has rank
greater or equal to the rank of the type of the other operand, then
the operand with signed integer type is converted to the type of the
operand with unsigned integer type.
That verbiage was adopted with c11, IIRC - and I think it means that the
compiler would implicitly convert the types correctly (rather than the
older conversion rules that would have resulted in the -1 being
UINT_MAX, again assuming I'm reading it correctly).
> 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 <echau...@redhat.com>
> ---
> lib/daemon-unix.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 4fdc6e3c4..78a013645 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,19 @@ get_sysconf_buffer_size(void)
> VLOG_FATAL("%s: Read initial passwordd struct size "
> "failed (%s), aborting. ", pidfile,
> ovs_strerror(errno));
> + } else {
> + 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 {
> + grp_bs = default_bufsize;
> }
> }
Won't the snippet at the bottom handle the else cases here:
bufsize = MAX(pwd_bs, grp_bs);
return bufsize ? bufsize : default_bufsize;
The rest of the code that uses get_pwname_r / get_pwuid_r also will loop
through correctly if the buffersize is unsufficient.
If I understand correcly, could this all be avoided by casting the -1 to
size_t in the 'if' conditions?
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
