Re: [PATCH 3/3] can: enable memcg accounting for user triggered allocations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] can: j1939: non-zero setsockopt return value in
  j1939_sk_setsockopt_flag() [1]
--

commit b3ee6688711331e68a7e275f9b72d5b3fa8b7ee5
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: enable memcg accounting for user triggered allocations

This commit enables memcg accounting via GFP_KERNEL_ACCOUNT and
SLAB_ACCOUNT for user-triggered allocations in the CAN core, BCM, ISOTP,
J1939, and RAW socket protocols.

Link: https://lore.kernel.org/linux-can/350df96b-5f69-490e-b39e-ed6e9bca4754@hartkopp.net/

> diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
> index 50a598ef5fd4a..c832465876049 100644
> --- a/net/can/j1939/socket.c
> +++ b/net/can/j1939/socket.c
> @@ -716,9 +716,14 @@ static int j1939_sk_setsockopt(struct socket *sock, int level, int optname,
>  				return -EINVAL;
>  
>  			count = optlen / sizeof(*filters);
> -			filters = memdup_sockptr(optval, optlen);
> -			if (IS_ERR(filters))
> -				return PTR_ERR(filters);
> +			filters = kmalloc(optlen, GFP_KERNEL_ACCOUNT);
> +			if (!filters)
> +				return -ENOMEM;
> +
> +			if (copy_from_sockptr(filters, optval, optlen)) {
> +				kfree(filters);
> +				return -EFAULT;
> +			}
>  
>  			for (f = filters, c = count; c; f++, c--) {
>  				f->name &= f->name_mask;

[Severity: Medium]
This problem wasn't introduced by this patch, but should
j1939_sk_setsockopt_flag() return 0 instead of tmp [1]?

net/can/j1939/socket.c:j1939_sk_setsockopt_flag() {
    ...
	lock_sock(&jsk->sk);
	if (tmp)
		jsk->state |= flag;
	else
		jsk->state &= ~flag;
	release_sock(&jsk->sk);
	return tmp;
}

When userspace enables SO_J1939_PROMISC or SO_J1939_ERRQUEUE with a
non-zero value in j1939_sk_setsockopt(), that value is returned to
userspace from setsockopt(), breaking callers that check for a 0 return on
success.

[1] https://sashiko.dev/#/bug/linux-35e22274-4165-4b2b-985e-c5f5773b9ef9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-can-bcmlock-memcg-v1-0-482d9f5be1fd@hartkopp.net?part=3