On Sun, Feb 19, 2017 at 02:47:02PM +0200, Matan Barak wrote:
> >>
> >> All these callbacks are assumed to run when the context is alive.
> >
> > 'uboject->context' is touched outside the SRCU, eg in
> > uverbs_close_fd() - so it absolutely needs locking.
> >
>
> uobject->context isn't protected by SRCU as intended.
> lookup_get_fd_uobject can't be executed concurrently with
> uverbs_close_fd, as uverbs_close_fd is called when the last
> reference of the fd is put and in that case fget called by
> lookup_get_fd_uobject would simply fail. Since fput and fget are
> atomically safe, this should be safe.
That is so tricky it needs a comment, but yes, that argument makes
sense for uverbs_close_fd().
But, you've just explained away every reason for this test:
+ if (f->f_op != fd_type->fops ||
+ !uobject->context) {
It should probably be
if (f->f_op != fd_type->fops) {
fput(f);
return ERR_PTR(-EBADF);
}
/* fget(id) ensures we are not currently running
* uverbs_close_fd, and the caller is expected to ensure
* that uverbs_cleanup_ucontext is never done while
* a call top lookup is possible. */
WARN_ON(!uobject->context);
if (!uobject->context) {
// Try to recover
fput(f);
return ERR_PTR(-EBADF);
}
Or you need to explain where context = null comes from..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
