Re: [PATCH -next] tcp: fix a signed-integer-overflow bug in tcp_add_backlog()

[luwei (O)]

在 2022/10/12 8:31 PM, Eric Dumazet 写道:
> On Wed, Oct 12, 2022 at 2:35 AM Lu Wei <luwei32@xxxxxxxxxx> wrote:
> > The type of sk_rcvbuf and sk_sndbuf in struct sock is int, and
> > in tcp_add_backlog(), the variable limit is caculated by adding
> > sk_rcvbuf, sk_sndbuf and 64 * 1024, it may exceed the max value
> > of u32 and be truncated. So change it to u64 to avoid a potential
> > signed-integer-overflow, which leads to opposite result is returned
> > in the following function.
> >
> > Signed-off-by: Lu Wei <luwei32@xxxxxxxxxx>
> You need to add a Fixes: tag, please.
>
> > ---
> >   include/net/sock.h  | 4 ++--
> >   net/ipv4/tcp_ipv4.c | 6 ++++--
> >   2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 08038a385ef2..fc0fa29d8865 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1069,7 +1069,7 @@ static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
> >    * Do not take into account this skb truesize,
> >    * to allow even a single big packet to come.
> >    */
> > -static inline bool sk_rcvqueues_full(const struct sock *sk, unsigned int limit)
> > +static inline bool sk_rcvqueues_full(const struct sock *sk, u64 limit)
> >   {
> >          unsigned int qsize = sk->sk_backlog.len + atomic_read(&sk->sk_rmem_alloc);
> qsize would then overflow :/
>
> I would rather limit sk_rcvbuf and sk_sndbuf to 0x7fff0000, instead of
> 0x7ffffffe
>
> If really someone is using 2GB for both send and receive queues,  I
> doubt removing 64KB will be a problem.
> .

thanks for reply, I will change the type of qsize to u64 in V2. Besides, 
how to limit sk_rcvbuf and sk_sndbuf

to 0x7ffff0000, do you mean in sysctl interface? If so, the varible 
limit will still overflow since it's calculated

by adding sk_rcvbuf and sk_sndbuf.

--
Best Regards,
Lu Wei