On Thu, Jun 2, 2016 at 2:23 PM, Eric W. Biederman <ebied...@xmission.com> wrote: > > We certainly don't need the permission check.
I agree that we don't need it, but it does simplify the code, and it doesn't actually harm anything, since the thing we check for permissions is the same /dev directory, which we already know we had lookup permissions for for the original ptmx node anyway. The permission checks I didn't like were the permission checks for the pts/ and pts/ptmx nodes, because they had nothing to do with the permission for the original node. In contrast, this is just about a redundant permission check we already did, and that simplifies the code a bit. That said, you're right that it needs to use IS_ERR(path.dentry). And the whole "lookup_one_len_unlocked()" simplification is just a small detail that doesn't much matter, so it's not a big deal. And it is arguable that "d_hash_and_lookup()" that you use is actually simpler than "lookup_one_len_unlocked()", since we are really only need to look into the dentry cache, and that's all that d_hash_and_lookup() does (ie it only ever does a d_lookup(), while lookup_one_len_unlocked() ends up doing a full lookup in the failuer case that we don't even care about. So I don't feel very strongly about it. Your patch is ok by me. Linus