On Tue, Oct 27, 2020 at 05:17:58PM +0300, Dan Carpenter wrote:
> There are three changes but none of them should affect run time:
>
> 1) You can't write to this file because the permissions are 0444. But
> it sort of looked like you could do a write and it would result in
> a read. Then it looked like proc_sys_call_handler() just ignored
> it. Which is confusing. It's more clear if the "write" just
> returns zero.
> 2) The "lenp" pointer is never NULL so that check can be removed.
> 3) In the original code, the "if (*lenp < 0)" check didn't work because
> "*lenp" is unsigned. Fortunately, the memory_read_from_buffer()
> call will never fail in this context so it doesn't affect runtime.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> net/sunrpc/sysctl.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
> index a18b36b5422d..04526bab4a06 100644
> --- a/net/sunrpc/sysctl.c
> +++ b/net/sunrpc/sysctl.c
> @@ -63,19 +63,19 @@ static int proc_do_xprt(struct ctl_table *table, int write,
> void *buffer, size_t *lenp, loff_t *ppos)
> {
> char tmpbuf[256];
> - size_t len;
> + ssize_t len;
>
> - if ((*ppos && !write) || !*lenp) {
> - *lenp = 0;
> + *lenp = 0;
> +
> + if (write || *ppos)
> return 0;
> - }
> +
> len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
> - *lenp = memory_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
> + len = memory_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
Except now we're passing *lenp = 0, that can't be right.
Though I actually kind of prefer this to Colin King's patch which just
casts (*lenp) in the comparison below.
--b.
> + if (len < 0)
> + return len;
>
> - if (*lenp < 0) {
> - *lenp = 0;
> - return -EINVAL;
> - }
> + *lenp = len;
> return 0;
> }
>
> --
> 2.28.0
