netdev - Re: [PATCH] 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]

Date: Tue, 20 Feb 2024 09:37:01 +0100
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Eric Dumazet <edumazet@...gle.com>, Marc Kleine-Budde <mkl@...gutronix.de>
Cc: "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] can: raw: raw_getsockopt(): reduce scope of err



On 2024-02-20 09:25, 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 ?
> 
> 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 ?

The patch with the fix has been sent yesterday and Marc applied it to 
his linux-can-next tree for a pull request to net-next.

https://lore.kernel.org/linux-can/20240220-mobility-thigh-8ddfb02bfab9-mkl@pengutronix.de/T/#t

Best regards,
Oliver

> 
> 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))
>