can: raw: add locking for raw flags bitfield

With commit 890e5198a6e5 ("can: raw: use bitfields to store flags in
struct raw_sock") the formerly separate integer values have been integrated
into a single bitfield. This led to a read-modify-write operation when
changing a flag in raw_setsockopt() which now needs a locking to prevent
concurrent access.

Instead of adding a lock/unlock hell in each of the flag manipulations this
patch introduces a wrapper for a new raw_setsockopt_locked() function
analogue to the isotp_setsockopt[_locked]() approach in net/can/isotp.c

Fixes: 890e5198a6e5 ("can: raw: use bitfields to store flags in struct raw_sock")
Reported-by: Eulgyu Kim <eulgyukim@snu.ac.kr>
Closes: https://lore.kernel.org/linux-can/20260503112200.22727-1-eulgyukim@snu.ac.kr/
Tested-by: Eulgyu Kim <eulgyukim@snu.ac.kr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Reviewed-by: Vincent Mailhol <mailhol@kernel.org>
Tested-by: Vincent Mailhol <mailhol@kernel.org>
Link: https://patch.msgid.link/20260504111928.41856-1-socketcan@hartkopp.net
[mkl: use Closes tag instead of Link]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/net/can/raw.c b/net/can/raw.c
index a26942e78e688721c9306f4e3fcbf25cd395a127..82d9c0499c95facbfde2d5593df650b2e5fe93a4 100644 (file)
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -562,8 +562,8 @@ static int raw_getname(struct socket *sock, struct sockaddr *uaddr,
        return RAW_MIN_NAMELEN;
 }
 
-static int raw_setsockopt(struct socket *sock, int level, int optname,
-                         sockptr_t optval, unsigned int optlen)
+static int raw_setsockopt_locked(struct socket *sock, int optname,
+                                sockptr_t optval, unsigned int optlen)
 {
        struct sock *sk = sock->sk;
        struct raw_sock *ro = raw_sk(sk);
@@ -575,9 +575,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
        int flag;
        int err = 0;
 
-       if (level != SOL_CAN_RAW)
-               return -EINVAL;
-
        switch (optname) {
        case CAN_RAW_FILTER:
                if (optlen % sizeof(struct can_filter) != 0)
@@ -598,17 +595,11 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
                                return -EFAULT;
                }
 
-               rtnl_lock();
-               lock_sock(sk);
-
                dev = ro->dev;
-               if (ro->bound && dev) {
-                       if (dev->reg_state != NETREG_REGISTERED) {
-                               if (count > 1)
-                                       kfree(filter);
-                               err = -ENODEV;
-                               goto out_fil;
-                       }
+               if (ro->bound && dev && dev->reg_state != NETREG_REGISTERED) {
+                       if (count > 1)
+                               kfree(filter);
+                       return -ENODEV;
                }
 
                if (ro->bound) {
@@ -622,7 +613,7 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
                        if (err) {
                                if (count > 1)
                                        kfree(filter);
-                               goto out_fil;
+                               return err;
                        }
 
                        /* remove old filter registrations */
@@ -642,11 +633,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
                }
                ro->filter = filter;
                ro->count  = count;
-
- out_fil:
-               release_sock(sk);
-               rtnl_unlock();
-
                break;
 
        case CAN_RAW_ERR_FILTER:
@@ -658,16 +644,9 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
 
                err_mask &= CAN_ERR_MASK;
 
-               rtnl_lock();
-               lock_sock(sk);
-
                dev = ro->dev;
-               if (ro->bound && dev) {
-                       if (dev->reg_state != NETREG_REGISTERED) {
-                               err = -ENODEV;
-                               goto out_err;
-                       }
-               }
+               if (ro->bound && dev && dev->reg_state != NETREG_REGISTERED)
+                       return -ENODEV;
 
                /* remove current error mask */
                if (ro->bound) {
@@ -676,7 +655,7 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
                                                   err_mask);
 
                        if (err)
-                               goto out_err;
+                               return err;
 
                        /* remove old err_mask registration */
                        raw_disable_errfilter(sock_net(sk), dev, sk,
@@ -685,11 +664,6 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
 
                /* link new err_mask to the socket */
                ro->err_mask = err_mask;
-
- out_err:
-               release_sock(sk);
-               rtnl_unlock();
-
                break;
 
        case CAN_RAW_LOOPBACK:
@@ -769,6 +743,26 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
        return err;
 }
 
+static int raw_setsockopt(struct socket *sock, int level, int optname,
+                         sockptr_t optval, unsigned int optlen)
+{
+       struct sock *sk = sock->sk;
+       int err;
+
+       if (level != SOL_CAN_RAW)
+               return -EINVAL;
+
+       rtnl_lock();
+       lock_sock(sk);
+
+       err = raw_setsockopt_locked(sock, optname, optval, optlen);
+
+       release_sock(sk);
+       rtnl_unlock();
+
+       return err;
+}
+
 static int raw_getsockopt(struct socket *sock, int level, int optname,
                          sockopt_t *opt)
 {