[bug report] net/sunrpc: Fix return value for sysctl sunrpc.transports

[bug report] net/sunrpc: Fix return value for sysctl sunrpc.transports

[Dan Carpenter]

Hello Artur Molchanov,

The patch c09f56b8f68d: "net/sunrpc: Fix return value for sysctl
sunrpc.transports" from Oct 12, 2020, leads to the following static
checker warning:

	net/sunrpc/sysctl.c:75 proc_do_xprt()
	warn: unsigned '*lenp' is never less than zero.

net/sunrpc/sysctl.c
    62  static int proc_do_xprt(struct ctl_table *table, int write,
    63                          void *buffer, size_t *lenp, loff_t *ppos)
    64  {
    65          char tmpbuf[256];
    66          size_t len;
    67  
    68          if ((*ppos && !write) || !*lenp) {
                              ^^^^^^^
It's weird that we don't just return -EINVAL for writes or something.

    69                  *lenp = 0;
    70                  return 0;
    71          }
    72          len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
    73          *lenp = memory_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
    74  
    75          if (*lenp < 0) {

"*lenp" is unsigned so it can't be less than zero.  memory_read_from_buffer()
only returns an error if ppos is negative but that's impossible because
this is a proc file so negatives are prevented in rw_verify_area().

In other words this bug can't affect runtime.

    76                  *lenp = 0;
    77                  return -EINVAL;
    78          }
    79          return 0;
    80  }

regards,
dan carpenter

Re: [bug report] net/sunrpc: Fix return value for sysctl sunrpc.transports

[Artur Molchanov]

Hello Dan,

Please ensure that my patch primarily fixes returning value of the function proc_do_xprt().
Before this patch proc_do_xprt() returns output of memory_read_from_buffer().
If memory_read_from_buffer() returns non-zero value then output of sysctl would remains uninitialized.

If memory_read_from_buffer() can not returns the negative values then we could just get rid of the condition.


Artur Molchanov

> 27 окт. 2020 г., в 11:31, Dan Carpenter <dan.carpenter@oracle.com> написал(а):
> 
> Hello Artur Molchanov,
> 
> The patch c09f56b8f68d: "net/sunrpc: Fix return value for sysctl
> sunrpc.transports" from Oct 12, 2020, leads to the following static
> checker warning:
> 
> 	net/sunrpc/sysctl.c:75 proc_do_xprt()
> 	warn: unsigned '*lenp' is never less than zero.
> 
> net/sunrpc/sysctl.c
>    62  static int proc_do_xprt(struct ctl_table *table, int write,
>    63                          void *buffer, size_t *lenp, loff_t *ppos)
>    64  {
>    65          char tmpbuf[256];
>    66          size_t len;
>    67  
>    68          if ((*ppos && !write) || !*lenp) {
>                              ^^^^^^^
> It's weird that we don't just return -EINVAL for writes or something.
> 
>    69                  *lenp = 0;
>    70                  return 0;
>    71          }
>    72          len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
>    73          *lenp = memory_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
>    74  
>    75          if (*lenp < 0) {
> 
> "*lenp" is unsigned so it can't be less than zero.  memory_read_from_buffer()
> only returns an error if ppos is negative but that's impossible because
> this is a proc file so negatives are prevented in rw_verify_area().
> 
> In other words this bug can't affect runtime.
> 
>    76                  *lenp = 0;
>    77                  return -EINVAL;
>    78          }
>    79          return 0;
>    80  }
> 
> regards,
> dan carpenter