Received-SPF: pass (google.com: domain of linux-kernel+bounces-72577-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Tue, 20 Feb 2024 09:37:46 +0100
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Eric Dumazet <edumazet@google.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>, 
	"David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, 
	Paolo Abeni <pabeni@redhat.com>, linux-can@vger.kernel.org, netdev@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] can: raw: raw_getsockopt(): reduce scope of err
Message-ID: <20240220-twig-dean-11e4f07f52b6-mkl@pengutronix.de>
References: <20240220-raw-setsockopt-v1-1-7d34cb1377fc@pengutronix.de>
 <CANn89iL7N-1zvBBLoz0qhCApVJRF1LKu=jSyC0yz-wHa3JLGNg@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="stun4zbvotcra547"
Content-Disposition: inline
In-Reply-To: <CANn89iL7N-1zvBBLoz0qhCApVJRF1LKu=jSyC0yz-wHa3JLGNg@mail.gmail.com>


--stun4zbvotcra547
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 20.02.2024 09:25:58, Eric Dumazet wrote:
> On Tue, Feb 20, 2024 at 9:16=E2=80=AFAM Marc Kleine-Budde <mkl@pengutroni=
x.de> wrote:
> >
> > Reduce the scope of the variable "err" to the individual cases. This
> > is to avoid the mistake of setting "err" in the mistaken belief that
> > it will be evaluated later.
> >
> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > ---
> >  net/can/raw.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/can/raw.c b/net/can/raw.c
> > index 897ffc17d850..2bb3eab98af0 100644
> > --- a/net/can/raw.c
> > +++ b/net/can/raw.c
> > @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int =
level, int optname,
> >         struct raw_sock *ro =3D raw_sk(sk);
> >         int len;
> >         void *val;
> > -       int err =3D 0;
> >
> >         if (level !=3D SOL_CAN_RAW)
> >                 return -EINVAL;
> > @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int =
level, int optname,
> >                 return -EINVAL;
> >
> >         switch (optname) {
> > -       case CAN_RAW_FILTER:
> > +       case CAN_RAW_FILTER: {
> > +               int err =3D 0;
> > +
> >                 lock_sock(sk);
> >                 if (ro->count > 0) {
> >                         int fsize =3D ro->count * sizeof(struct can_fil=
ter);
> > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int =
level, int optname,
> >                 if (!err)
> >                         err =3D put_user(len, optlen);
> >                 return err;
> > -
> > +       }
> >         case CAN_RAW_ERR_FILTER:
> >                 if (len > sizeof(can_err_mask_t))
> >                         len =3D sizeof(can_err_mask_t);
> > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int =
level, int optname,
> >                 val =3D &ro->xl_frames;
> >                 break;
> >
> > -       case CAN_RAW_XL_VCID_OPTS:
> > +       case CAN_RAW_XL_VCID_OPTS: {
> > +               int err =3D 0;
> > +
> >                 /* user space buffer to small for VCID opts? */
> >                 if (len < sizeof(ro->raw_vcid_opts)) {
> >                         /* return -ERANGE and needed space in optlen */
> > @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int =
level, int optname,
> >                         err =3D put_user(len, optlen);
> >                 return err;
> >
> > +       }
> >         case CAN_RAW_JOIN_FILTERS:
> >                 if (len > sizeof(int))
> >                         len =3D sizeof(int);
> >
> > ---
> > base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904
> > change-id: 20240220-raw-setsockopt-f6e173cdbbbb
>=20
> What is the target tree ?

Sorry I have to fine tune my b4 setup, this targets net-next. And
depends on https://lore.kernel.org/all/20240219200021.12113-1-socketcan@har=
tkopp.net/

> In net-next tree, syzbot complained about a bug added in
>=20
> commit c83c22ec1493c0b7cc77327bedbd387e295872b6
> Author: Oliver Hartkopp <socketcan@hartkopp.net>
> Date:   Mon Feb 12 22:35:50 2024 +0100
>=20
>     can: canxl: add virtual CAN network identifier support
>=20
> Patch to fix the issue has not been sent yet ?
>=20
> diff --git a/net/can/raw.c b/net/can/raw.c
> index cb8e6f788af84ac65830399baac6d1cf3d093e08..897ffc17d850670003e5cf340=
2477e8fc201f61e
> 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -835,7 +835,9 @@ static int raw_getsockopt(struct socket *sock, int
> level, int optname,
>                         if (copy_to_user(optval, &ro->raw_vcid_opts, len))
>                                 err =3D -EFAULT;
>                 }
> -               break;
> +               if (!err)
> +                       err =3D put_user(len, optlen);
> +               return err;
>=20
>         case CAN_RAW_JOIN_FILTERS:
>                 if (len > sizeof(int))
>

The above mentioned patch fixes the issue introduced in c83c22ec1493
("can: canxl: add virtual CAN network identifier support").

regards,
Marc

--=20
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung N=C3=BCrnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

--stun4zbvotcra547
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEUEC6huC2BN0pvD5fKDiiPnotvG8FAmXUZNcACgkQKDiiPnot
vG89EAf/SUME5QqEm+81Zh004IMgC8KZ4Nhv0diZXK1WVCy7XzQ/1dDkmS4A35Gp
o0zsuz/+tu35EPSt7/XoqaLlV5zwcxSNk+pmq9EdVq7hcX8dz4VujINdLQ+qlDv+
Fy+4V7vG3A9J0js5fS8bei1AjlByT34X+YPQ/CkVrzcU6DyxqdO2OVPJg+ZvP023
YiCrSjlvZpUkhNTd34Npqrk7lec14eEmStRa8Zo5Z01v3iZJxVGLnbVpRknZbujp
cES/ejGfhxjnC159oaijrhAKYd/mTB9OM5vxKWJO67SQTOGqmkWp9GfSHobSY+E2
4QyUFbHMiTh695ynSGk7qqT2cr/bzQ==
=Vdbz
-----END PGP SIGNATURE-----

--stun4zbvotcra547--