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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240220-twig-dean-11e4f07f52b6-mkl@pengutronix.de>
Date: Tue, 20 Feb 2024 09:37:46 +0100
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Oliver Hartkopp <socketcan@...tkopp.net>, 
	"David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, linux-can@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] can: raw: raw_getsockopt(): reduce scope of err

On 20.02.2024 09:25:58, Eric Dumazet wrote:
> On Tue, Feb 20, 2024 at 9:16 AM Marc Kleine-Budde <mkl@...gutronix.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@...gutronix.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 = 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@hartkopp.net/

> In net-next tree, syzbot complained about a bug added in
> 
> commit c83c22ec1493c0b7cc77327bedbd387e295872b6
> Author: Oliver Hartkopp <socketcan@...tkopp.net>
> 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   |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)