Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\))
Subject: Re: [bug report] net/sunrpc: Fix return value for sysctl
 sunrpc.transports
From:   Artur Molchanov <arturmolchanov@gmail.com>
In-Reply-To: <20201027083158.GA2533809@mwanda>
Date:   Tue, 27 Oct 2020 11:51:53 +0300
Cc:     linux-nfs@vger.kernel.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <0686EA90-655B-4682-9CA8-0C48B29DC685@gmail.com>
References: <20201027083158.GA2533809@mwanda>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Precedence: bulk

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 =D0=BE=D0=BA=D1=82. 2020 =D0=B3., =D0=B2 11:31, Dan Carpenter =
<dan.carpenter@oracle.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0=
):
>=20
> Hello Artur Molchanov,
>=20
> The patch c09f56b8f68d: "net/sunrpc: Fix return value for sysctl
> sunrpc.transports" from Oct 12, 2020, leads to the following static
> checker warning:
>=20
> 	net/sunrpc/sysctl.c:75 proc_do_xprt()
> 	warn: unsigned '*lenp' is never less than zero.
>=20
> 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 =20
>    68          if ((*ppos && !write) || !*lenp) {
>                              ^^^^^^^
> It's weird that we don't just return -EINVAL for writes or something.
>=20
>    69                  *lenp =3D 0;
>    70                  return 0;
>    71          }
>    72          len =3D svc_print_xprts(tmpbuf, sizeof(tmpbuf));
>    73          *lenp =3D memory_read_from_buffer(buffer, *lenp, ppos, =
tmpbuf, len);
>    74 =20
>    75          if (*lenp < 0) {
>=20
> "*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().
>=20
> In other words this bug can't affect runtime.
>=20
>    76                  *lenp =3D 0;
>    77                  return -EINVAL;
>    78          }
>    79          return 0;
>    80  }
>=20
> regards,
> dan carpenter