netdev - Re: [PATCH] tcp: detect use sendpage for slab-based objects

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] [day] [month] [year] [list]

Message-ID: <c689e501-4dbf-a569-d64b-ab9b7ca52007@virtuozzo.com>
Date:   Tue, 5 Mar 2019 21:35:33 +0300
From:   Vasily Averin <vvs@...tuozzo.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        netdev <netdev@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] tcp: detect use sendpage for slab-based objects



On 3/5/19 7:44 PM, Eric Dumazet wrote:
> On Tue, Mar 5, 2019 at 7:11 AM Eric Dumazet <edumazet@...gle.com> wrote:
>>> My original suggestion was to use VM_WARN_ONCE() so that the debug checks would
>>> be compiled out by the compiler, unless you compile a debug kernel.
>>>
>>> Something like :
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index ad07dd71063da09843ccfbd3e00d3f41567e1205..889563a66dde2a41c198d92a80183cf5382f632d 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -943,6 +943,9 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
>>>         ssize_t copied;
>>>         long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
>>>
>>> +       if (VM_WARN_ONCE(PageSlab(page)), "page must not be a Slab one")
>>> +               return -EINVAL;
>>> +
> 
> Well, VM_WARN_ONCE() returns 1 if !CONFIG_DEBUG_VM,
> so it is not behaving like WARN_ONCE(), which is unfortunate.
> 
> But you get the idea ;)

Thanks, I did not know this trick. 
Seems it can be replaced by following construction, I'll check it tomorrow morning.

#ifdef CONFIG_DEBUG_VM
	if (WARN_ONCE(PageSlab(page), "page must not be a Slab one")
		return -EINVAL;
#endif

I expect it should affect debug kernels only, 
fail all incorrect requests, taint the kernel and generate warning only once.

>>>          * (passive side) where data is allowed to be sent before a connection
>>>          * is fully established.
>>>
>>>
>>> For some reason you added a fallback :/
>>>
>