Re: [PATCH net-next] can: raw: raw_getsockopt(): reduce scope of err

From: Marc Kleine-Budde
Date: Tue Feb 20 2024 - 03:38:37 EST


On 20.02.2024 09:25:58, Eric Dumazet wrote:
> On Tue, Feb 20, 2024 at 9:16 AM Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > ---
> > 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 = raw_sk(sk);
> > int len;
> > void *val;
> > - int err = 0;
> >
> > if (level != 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 = 0;
> > +
> > lock_sock(sk);
> > if (ro->count > 0) {
> > int fsize = ro->count * sizeof(struct can_filter);
> > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
> > if (!err)
> > err = put_user(len, optlen);
> > return err;
> > -
> > + }
> > case CAN_RAW_ERR_FILTER:
> > if (len > sizeof(can_err_mask_t))
> > len = sizeof(can_err_mask_t);
> > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
> > val = &ro->xl_frames;
> > break;
> >
> > - case CAN_RAW_XL_VCID_OPTS:
> > + case CAN_RAW_XL_VCID_OPTS: {
> > + int err = 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 = put_user(len, optlen);
> > return err;
> >
> > + }
> > case CAN_RAW_JOIN_FILTERS:
> > if (len > sizeof(int))
> > len = sizeof(int);
> >
> > ---
> > base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904
> > change-id: 20240220-raw-setsockopt-f6e173cdbbbb
>
> 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@xxxxxxxxxxxx/

> In net-next tree, syzbot complained about a bug added in
>
> commit c83c22ec1493c0b7cc77327bedbd387e295872b6
> Author: Oliver Hartkopp <socketcan@xxxxxxxxxxxx>
> Date: Mon Feb 12 22:35:50 2024 +0100
>
> can: canxl: add virtual CAN network identifier support
>
> Patch to fix the issue has not been sent yet ?
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index cb8e6f788af84ac65830399baac6d1cf3d093e08..897ffc17d850670003e5cf3402477e8fc201f61e
> 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 = -EFAULT;
> }
> - break;
> + if (!err)
> + err = put_user(len, optlen);
> + return err;
>
> 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

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

Attachment: signature.asc
Description: PGP signature