Re: [PATCH] io_uring: add overflow checks for poll refcounting

[Jens Axboe <axboe@kernel.dk>]

On 3/23/22 5:14 AM, Pavel Begunkov wrote:
> We already got one bug with ->poll_refs overflows, let's add overflow
> checks for it in a similar way as we do for request refs. For that
> reserve the sign bit so underflows don't set IO_POLL_CANCEL_FLAG and
> making us able to catch them.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 245610494c3e..594ed8bc4585 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5803,8 +5803,13 @@ struct io_poll_table {
>  	int error;
>  };
>  
> -#define IO_POLL_CANCEL_FLAG	BIT(31)
> -#define IO_POLL_REF_MASK	GENMASK(30, 0)
> +/* keep the sign bit unused to improve overflow detection */
> +#define IO_POLL_CANCEL_FLAG	BIT(30)
> +#define IO_POLL_REF_MASK	GENMASK(29, 0)
> +
> +/* 2^16 is choosen arbitrary, would be funky to have more than that */
> +#define io_poll_ref_check_overflow(refs) ((unsigned int)refs >= 65536u)
> +#define io_poll_ref_check_underflow(refs) ((int)refs < 0)

Should that be larger? I agree that it'd be funky to have > 64k, but we
just had such a case with remove all which triggered this whole thing.
That case actually would've worked fine, albeit slower, if we weren't
limited to 20 bits of refs at that time. Maybe just trigger when we're
halfway there, AND'ing with bit 29?

-- 
Jens Axboe