On Fri, Oct 11, 2024 at 8:27 AM Jakub Kicinski [off-list ref] wrote:

On Thu, 10 Oct 2024 12:05:38 -0700 Mina Almasry wrote:

quoted

diff --git a/net/core/sock.c b/net/core/sock.c
index 083d438d8b6f..cb3d8b19de14 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

@@ -1071,11 +1071,11 @@ sock_devmem_dontneed(struct sock *sk,

sockptr_t optval, unsigned int optlen)
            optlen > sizeof(*tokens) * MAX_DONTNEED_TOKENS)
                return -EINVAL;

-       tokens = kvmalloc_array(optlen, sizeof(*tokens), GFP_KERNEL);
+       num_tokens = optlen / sizeof(struct dmabuf_token);
+       tokens = kvmalloc_array(num_tokens, sizeof(*tokens), GFP_KERNEL);
        if (!tokens)
                return -ENOMEM;

-       num_tokens = optlen / sizeof(struct dmabuf_token);
        if (copy_from_sockptr(tokens, optval, optlen)) {
                kvfree(tokens);
                return -EFAULT;

@@ -1083,6 +1083,10 @@ sock_devmem_dontneed(struct sock *sk, sockptr_t

optval, unsigned int optlen)

        xa_lock_bh(&sk->sk_user_frags);
        for (i = 0; i < num_tokens; i++) {
+
+               if (tokens[i].token_count > MAX_DONTNEED_TOKENS)
+                       continue;

For the real fix let's scan the tokens before we take the xa lock
and return an error rather than silently skipping?

quoted

                for (j = 0; j < tokens[i].token_count; j++) {

Yes, sorry, I called the diff above an 'untested fix' but it was more
of a hack to see if I got the root cause right. For a proper fix, we
should do exactly that. Scan and see how many tokens the user is
asking us to free ahead of time, then exit early if it's too much
before we acquire locks and loop. Will do!

-- 
Thanks,
Mina