Re: [PATCH 1/2] io_uring: fix UAF for personality_idr

[Pavel Begunkov <asml.silence@gmail.com>]

On 08/03/2021 13:20, Matthew Wilcox wrote:
> On Mon, Mar 08, 2021 at 10:46:37AM +0000, Pavel Begunkov wrote:
>> Matthew, any chance you remember whether idr_for_each tolerates
>> idr_remove() from within the callback? Nothing else is happening in
>> parallel.
> 
> No, that's not allowed.  The design of the IDR is that you would free
> the thing being pointed to and then call idr_destroy() afterwards to

Gotcha, thanks!

> free the IDR's data structures.  But this should use an XArray anyway.
> Compile-tested only.

Yeah, I remember this patch, looks good but 1 comments below.
Anyway, I'll rebase and resend it shortly for convenience.

[...]
> @@ -9532,14 +9531,16 @@ static int io_probe(struct io_ring_ctx *ctx, void __user *arg, unsigned nr_args)
>  static int io_register_personality(struct io_ring_ctx *ctx)
>  {
>  	const struct cred *creds;
> +	u32 id;
>  	int ret;
>  
>  	creds = get_current_cred();
>  
> -	ret = idr_alloc_cyclic(&ctx->personality_idr, (void *) creds, 1,
> -				USHRT_MAX, GFP_KERNEL);
> -	if (ret < 0)
> -		put_cred(creds);
> +	ret = xa_alloc_cyclic(&ctx->personalities, &id, (void *)creds,
> +			XA_LIMIT(0, USHRT_MAX), &ctx->pers_next, GFP_KERNEL);

ids are >=1, because 0 is kind of a reserved value for io_uring, so I guess

XA_LIMIT(1, USHRT_MAX)

> +	if (!ret)
> +		return id;
> +	put_cred(creds);
>  	return ret;
>  }
>  
> 

-- 
Pavel Begunkov