Re: [PATCH 2/2] fsnotify: use method copy_dname copying filename

[Al Viro]

On Fri, Aug 04, 2017 at 11:58:41AM +0800, æåç wrote:
> Hi all
> 
> I sent this patch two months ago, then I found CVE from this link last night
> 
>     http://seclists.org/oss-sec/2017/q3/240
> 
> which not only references this patch, but also provides a upstream fix
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=49d31c2f389acfe83417083e1208422b4091cd9
> 
> I was wondering why @viro hadn't noticed this mail (And @viro fixed
> this). I'm new here and nobody,
> trying to do my best to help the this linux community. I was looking
> forword to your reply, because some
> insufficiency may exists in my work, I'd like to learn from you guys
> 
> Thanks and humble enough to wait your reply

Fair enough.  As for the reasons why allocation of name copy is a bad idea,
consider this: only short (embedded) names get overwritten on rename.
External ones (i.e. anything longer than 32 bytes or so) are unmodified
until freed.  And their lifetime is controlled by a refcount, so we can
trivially get a guaranteed to be stable name in that case - all it takes
is bumping the refcount and the name _will_ stay around until we drop
the reference.  So we are left with the case of short names and that
is trivial to deal with - 32-byte array is small enough, so we can bloody
well have it on caller's stack and copy the (short) name there.
That approach avoids all the headache with allocation, allocation failure
handling, etc.  Stack footprint is not much higher (especially compared
to how much idiotify and friends stomp on the stack) and it's obviously
cheaper - we only copy the name in short case and we never go into
allocator.  And it's just as easy to use as "make a dynamic copy" variant
of API...

Al, still buried in packing boxes at the moment...