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
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 <[email protected]> написал(а):
>
> 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