
| From: Alvin Starr via talk <talk@gtalug.org> | If the zdnet report is to be believed then There was at least one attempt to | insert code after being found out and asked to stop. | | https://www.zdnet.com/article/greg-kroah-hartman-bans-university-of-minnesot... See: <https://lore.kernel.org/linux-nfs/20210407001658.2208535-1-pakki001@umn.edu/> I don't think that Steven J. Vaughan-Nichols' interpretation is correct (it seems to be GKH's). If you look at the email exchange in question, the "attempt to insert code" was an attempt to submit a real bug-fix, not an attempt to add a bug. But: - the fix was to a bug that didn't exist. Careful reading of the surrounding code shows that the problem addressed could not happen. - it is hard to understand leaks and non-leaks, so this submission only shows that Pakki is not yet a good kernel programmer. - it does not introduce a vulnerability Here's the original function (from a perhaps different version of the kernel): static void gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) { struct gss_upcall_msg *gss_msg = container_of(msg, struct gss_upcall_msg, msg); if (msg->errno < 0) { refcount_inc(&gss_msg->count); gss_unhash_msg(gss_msg); if (msg->errno == -ETIMEDOUT) warn_gssd(); gss_release_msg(gss_msg); } gss_release_msg(gss_msg); } The patch submitted by Pakki was: --- a/net/sunrpc/auth_gss/auth_gss.c +++ b/net/sunrpc/auth_gss/auth_gss.c @@ -848,7 +848,8 @@ gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) warn_gssd(); gss_release_msg(gss_msg); } - gss_release_msg(gss_msg); + if (gss_msg) + gss_release_msg(gss_msg); } I don't see how gss_msg could be null, even just reading this code. So the added test doesn't change anything. No bug fixed. No bug introduced. This certainly doesn't add a vulnerability. But I think that the following code would work and be simpler. Note: my suggestion is just a guess. I don't know the semantics of the functions called. static void gss_pipe_destroy_msg(struct rpc_pipe_msg *msg) { struct gss_upcall_msg *gss_msg = container_of(msg, struct gss_upcall_msg, msg); if (msg->errno < 0) { gss_unhash_msg(gss_msg); if (msg->errno == -ETIMEDOUT) warn_gssd(); } gss_release_msg(gss_msg); } Something like this was suggested in the LKML thread.