Re: [PATCH v2 08/11] tcp: Remove unneccessary bounds check in tcp_timer_handler()

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 27 Nov 2023 10:33:45 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> In tcp_timer_handler() we use conn_at_idx() to interpret the flow index
> from the epoll reference.  However, this will never be NULL - we always
> put a valid index into the epoll_ref.  Simplify slightly based on this.

Sorry, I missed this during review of v1.

I have mixed feeling about this, and I don't think 11/11 changes
anything in this regard: we have to trust the kernel, as there's no
benefit to security in not doing so.

At the same time, should we ever get an out-of-bounds index from the
epoll event, we can fail gracefully here. Slightly worse, however: if
we get a timer event for a connection that's already closed, we'll
happily go and try to manipulate its timer (with or without the !conn
check).

All in all, I think the check is minimally useful, and we should have
something more robust than that. So if this patch helps keeping things
simple later in the series, I don't see an issue with that, but perhaps
we should add back a more sensible set of checks later.

The next patches all look good to me.

-- 
Stefano