On Sun, 24 Aug 2014 08:58:58 -0700 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > I like this change a lot! But one caveat: > > > + /* > > + * Despite the fact that it's an int return function, __f_setown never > > + * returns an error. Just ignore any error return here, but spew a > > + * WARN_ON_ONCE in case this ever changes. > > + */ > > + WARN_ON_ONCE(__f_setown(filp, task_pid(current), PIDTYPE_PID, 0)); > > I don't think this is a good idea. The errors in __f_setown come from > the security modules, and they could change easily. If you can convince > the LSM people to change their file_set_fowner routine to return void > we could change __f_setown to return void as well and be done with it, > but without that this looks too dangerous. > Understood. I figured that this might not be acceptable. I can make this propagate the error back up, but cleaning up the mess may not be easy. I'll see what I can do. Note that the error handling in the existing code looks wrong to me too. The lease gets added to the list (or updated), the fasync handler gets set up. Then, if __f_setown returns an error, the code just returns that error without unwinding anything. -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
