On Mon, Mar 01, 2021 at 05:44:02PM +0000, Chuck Lever wrote: > > So, the effect of this is to call svc_authorise more often. I think > > that's always safe, because svc_authorise is a no-op unless rq_authops > > is set, it clears rq_authops itself, and rq_authops being set is a > > guarantee that ->accept() already ran. > > > > It's harder to know if this solves the problem, as I see a lot of other > > mentions of THIS_MODULE in svcauth_gss.c. > > Perhaps a deeper audit is necessary. > > A small code change to inject SVC_CLOSE returns at random would enable > a more dynamic analysis. That might be interesting. I don't think tihs patch necessarily has to wait for that. > > Possibly orthogonal to this problem, but: svcauth_gss_release > > unconditionally dereferences rqstp->rq_auth_data. Isn't that a NULL > > dereference if the kmalloc at the start of svcauth_gss_accept() fails? > > > > Finally, should we care about module reference leaks? > > I would prefer that module reference counting work as expected. When it > doesn't that tends to lead to people (say, me) hunting for bugs that > might actually be serious. > > > > Does anyone really *need* to unload modules? > > Anyone who wants to replace the module with a newer build that fixes a > bug. It avoids a full reboot, and for some that's important. Fair enough. --b.
