On Tue, Feb 11, 2020 at 10:57 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > Or (just throwing another idea out there) the dentry's name could be copied to a > temporary buffer in ->d_compare(). The simplest version would be: > > u8 _name[NAME_MAX]; > > memcpy(_name, name, len); > name = _name; > > Though, 255 bytes is a bit large for a stack buffer (so for long names it may > need kmalloc with GFP_ATOMIC), and technically it would need a special version > of memcpy() to be guaranteed safe from compiler optimizations (though I expect > this would work in practice). > > Alternatively, take_dentry_name_snapshot() kind of does this already, except > that it takes a dentry and not a (name, len) pair. > > - Eric If we want to use take_dentry_name_snapshot, we'd need to do it before calling the dentry op, since we get the dentry as a const. It would do exactly what we want, in that it either takes a reference on the long name, or copies the short name, although it does so under a spinlock. I'm guessing we don't want to add that overhead for all d_compare/d_hash's. I suppose it could just take a snapshot if it falls under needs_casefold, but that feels a bit silly to me. i don't think utf8cursor/utf8byte could be modified to be RCU safe apart from a copy. As part of normalization there's some sorting that goes on to ensure that different encodings of the same characters can be matched, and I think those can technically be arbitrarily long, so we'd possibly end up needing the copy anyways. So, I see two possible fixes. 1. Use take_dentry_name_snapshot along the RCU paths to calling d_hash and d_compare, at least when needs_casefold is true. 2. Within d_hash/d_compare, create a copy of the name if it is a short name. For 1, it adds some overhead in general, which I'm sure we'd want to avoid. For 2, I don't think we know we're in RCU mode, so we'd need to always copy short filenames. I'm also unsure if it's valid to assume that name given is stable if it is not the same as dentry->d_iname. If it is, we only need to worry about copying DNAME_INLINE_LEN bytes at max there. For memcpy, is there a different version that we'd want to use for option 2? -Daniel
