From: Al Viro <viro@ZenIV.linux.org.uk>
To: 林守磊 <linxiulei@gmail.com>
Cc: 石祤 <leilei.lin@alibaba-inc.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
zhiche.yy@alibaba-inc.com, torvalds@linux-foundation.org,
linux-mm@kvack.org
Subject: Re: [PATCH 2/2] fsnotify: use method copy_dname copying filename
Date: Fri, 4 Aug 2017 06:18:16 +0100 [thread overview]
Message-ID: <20170804051816.GI2063@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CALPjY3mAV40cMD_iE=WVx2upxwgUYwBH-gdpgWY+RichywajfQ@mail.gmail.com>
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...
WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: 林守磊 <linxiulei@gmail.com>
Cc: 石祤 <leilei.lin@alibaba-inc.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
zhiche.yy@alibaba-inc.com, torvalds@linux-foundation.org,
linux-mm@kvack.org
Subject: Re: [PATCH 2/2] fsnotify: use method copy_dname copying filename
Date: Fri, 4 Aug 2017 06:18:16 +0100 [thread overview]
Message-ID: <20170804051816.GI2063@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CALPjY3mAV40cMD_iE=WVx2upxwgUYwBH-gdpgWY+RichywajfQ@mail.gmail.com>
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...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: 林守磊 <linxiulei@gmail.com>
Cc: 石祤 <leilei.lin@alibaba-inc.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
zhiche.yy@alibaba-inc.com, torvalds@linux-foundation.org,
linux-mm@kvack.org
Subject: Re: [PATCH 2/2] fsnotify: use method copy_dname copying filename
Date: Fri, 4 Aug 2017 06:18:16 +0100 [thread overview]
Message-ID: <20170804051816.GI2063@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CALPjY3mAV40cMD_iE=WVx2upxwgUYwBH-gdpgWY+RichywajfQ@mail.gmail.com>
On Fri, Aug 04, 2017 at 11:58:41AM +0800, ae??a(R)?cGBP? 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...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-08-04 5:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-31 3:54 [PATCH 0/2] fsnotify: fix mem overwritten 石祤
2017-05-31 3:54 ` [PATCH 1/2] fs/dcache.c: New copy_dname method 石祤
2017-05-31 3:54 ` [PATCH 2/2] fsnotify: use method copy_dname copying filename 石祤
2017-08-04 3:58 ` 林守磊
2017-08-04 3:58 ` 林守磊
2017-08-04 5:18 ` Al Viro [this message]
2017-08-04 5:18 ` Al Viro
2017-08-04 5:18 ` Al Viro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170804051816.GI2063@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=leilei.lin@alibaba-inc.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linxiulei@gmail.com \
--cc=torvalds@linux-foundation.org \
--cc=zhiche.yy@alibaba-inc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.