Re: [V9fs-developer] [PATCH 2/6] 9p: Replace the fidlist with an IDR

From: Matthew Wilcox
Date: Wed Jul 11 2018 - 09:09:02 EST


On Wed, Jul 11, 2018 at 02:58:12PM +0200, Dominique Martinet wrote:
> > I don't understand your assertion that this is a change of behaviour.
> > The implementation of p9_idpool_get() uses idr_alloc(), not
> > idr_alloc_cyclic(), so I don't believe I've changed which FID would
> > be allocated.
>
> Hmm, I just tried mounting something with ganesha and that broke because
> it received fid=1730858632 in a TATTACH request, so something in the
> patch series made some big fid happens.
>
> If you say this isn't intented though this needs debugging, I'm glad I
> brought this up :P
>
> (Note that it really should be fine if it is random within the pool, I
> have notified the current developpers of the problem)

Heh, unintended protocol fuzzing ;-)

I see the problem; need to initialise fid->fid to 0 before calling
idr_alloc_u32. I'd misread:

memset(&fid->qid, 0, sizeof(struct p9_qid));
as
memset(fid, 0, sizeof(struct p9_fid));

so I thought fid->fid was already 0. I'll fix that up, thanks!