| Message ID | 20200816070814.6806-1-colyli@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/3] net: introduce helper sendpage_ok() in include/linux/net.h | expand |
On 8/16/20 12:08 AM, Coly Li wrote: > The original problem was from nvme-over-tcp code, who mistakenly uses > kernel_sendpage() to send pages allocated by __get_free_pages() without > __GFP_COMP flag. Such pages don't have refcount (page_count is 0) on > tail pages, sending them by kernel_sendpage() may trigger a kernel panic > from a corrupted kernel heap, because these pages are incorrectly freed > in network stack as page_count 0 pages. > > This patch introduces a helper sendpage_ok(), it returns true if the > checking page, > - is not slab page: PageSlab(page) is false. > - has page refcount: page_count(page) is not zero > > All drivers who want to send page to remote end by kernel_sendpage() > may use this helper to check whether the page is OK. If the helper does > not return true, the driver should try other non sendpage method (e.g. > sock_no_sendpage()) to handle the page. > > > > diff --git a/include/linux/net.h b/include/linux/net.h > index d48ff1180879..a807fad31958 100644 > --- a/include/linux/net.h > +++ b/include/linux/net.h > @@ -21,6 +21,7 @@ > #include <linux/rcupdate.h> > #include <linux/once.h> > #include <linux/fs.h> > +#include <linux/mm.h> > #include <linux/sockptr.h> > > #include <uapi/linux/net.h> > @@ -286,6 +287,21 @@ do { \ > #define net_get_random_once_wait(buf, nbytes) \ > get_random_once_wait((buf), (nbytes)) > > +/* > + * E.g. XFS meta- & log-data is in slab pages, or bcache meta > + * data pages, or other high order pages allocated by > + * __get_free_pages() without __GFP_COMP, which have a page_count > + * of 0 and/or have PageSlab() set. We cannot use send_page for > + * those, as that does get_page(); put_page(); and would cause > + * either a VM_BUG directly, or __page_cache_release a page that > + * would actually still be referenced by someone, leading to some > + * obscure delayed Oops somewhere else. > + */ > +static inline bool sendpage_ok(struct page *page) > +{ > + return (!PageSlab(page) && page_count(page) >= 1); > +} > return (A); Can simply be written : return A; In this case : return !PageSlab(page) && page_count(page) >= 1; BTW, do you have plans to refine code added with commit a10674bf2406afc2554f9c7d31b2dc65d6a27fd9 ("tcp: detecting the misuse of .sendpage for Slab objects")
On 2020/8/18 16:08, Eric Dumazet wrote: > > > On 8/16/20 12:08 AM, Coly Li wrote: >> The original problem was from nvme-over-tcp code, who mistakenly uses >> kernel_sendpage() to send pages allocated by __get_free_pages() without >> __GFP_COMP flag. Such pages don't have refcount (page_count is 0) on >> tail pages, sending them by kernel_sendpage() may trigger a kernel panic >> from a corrupted kernel heap, because these pages are incorrectly freed >> in network stack as page_count 0 pages. >> >> This patch introduces a helper sendpage_ok(), it returns true if the >> checking page, >> - is not slab page: PageSlab(page) is false. >> - has page refcount: page_count(page) is not zero >> >> All drivers who want to send page to remote end by kernel_sendpage() >> may use this helper to check whether the page is OK. If the helper does >> not return true, the driver should try other non sendpage method (e.g. >> sock_no_sendpage()) to handle the page. >> >> >> >> diff --git a/include/linux/net.h b/include/linux/net.h >> index d48ff1180879..a807fad31958 100644 >> --- a/include/linux/net.h >> +++ b/include/linux/net.h >> @@ -21,6 +21,7 @@ >> #include <linux/rcupdate.h> >> #include <linux/once.h> >> #include <linux/fs.h> >> +#include <linux/mm.h> >> #include <linux/sockptr.h> >> >> #include <uapi/linux/net.h> >> @@ -286,6 +287,21 @@ do { \ >> #define net_get_random_once_wait(buf, nbytes) \ >> get_random_once_wait((buf), (nbytes)) >> >> +/* >> + * E.g. XFS meta- & log-data is in slab pages, or bcache meta >> + * data pages, or other high order pages allocated by >> + * __get_free_pages() without __GFP_COMP, which have a page_count >> + * of 0 and/or have PageSlab() set. We cannot use send_page for >> + * those, as that does get_page(); put_page(); and would cause >> + * either a VM_BUG directly, or __page_cache_release a page that >> + * would actually still be referenced by someone, leading to some >> + * obscure delayed Oops somewhere else. >> + */ >> +static inline bool sendpage_ok(struct page *page) >> +{ >> + return (!PageSlab(page) && page_count(page) >= 1); >> +} >> Hi Eric, > > return (A); > > Can simply be written : > > return A; > > In this case : > > return !PageSlab(page) && page_count(page) >= 1; Sure, I update it in v6 series. > > BTW, do you have plans to refine code added with commit a10674bf2406afc2554f9c7d31b2dc65d6a27fd9 > ("tcp: detecting the misuse of .sendpage for Slab objects") > Thanks for the hint, I will remove this piece in v6 series. Coly Li
diff --git a/include/linux/net.h b/include/linux/net.h index d48ff1180879..a807fad31958 100644 --- a/include/linux/net.h +++ b/include/linux/net.h @@ -21,6 +21,7 @@ #include <linux/rcupdate.h> #include <linux/once.h> #include <linux/fs.h> +#include <linux/mm.h> #include <linux/sockptr.h> #include <uapi/linux/net.h> @@ -286,6 +287,21 @@ do { \ #define net_get_random_once_wait(buf, nbytes) \ get_random_once_wait((buf), (nbytes)) +/* + * E.g. XFS meta- & log-data is in slab pages, or bcache meta + * data pages, or other high order pages allocated by + * __get_free_pages() without __GFP_COMP, which have a page_count + * of 0 and/or have PageSlab() set. We cannot use send_page for + * those, as that does get_page(); put_page(); and would cause + * either a VM_BUG directly, or __page_cache_release a page that + * would actually still be referenced by someone, leading to some + * obscure delayed Oops somewhere else. + */ +static inline bool sendpage_ok(struct page *page) +{ + return (!PageSlab(page) && page_count(page) >= 1); +} + int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec, size_t num, size_t len); int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
The original problem was from nvme-over-tcp code, who mistakenly uses kernel_sendpage() to send pages allocated by __get_free_pages() without __GFP_COMP flag. Such pages don't have refcount (page_count is 0) on tail pages, sending them by kernel_sendpage() may trigger a kernel panic from a corrupted kernel heap, because these pages are incorrectly freed in network stack as page_count 0 pages. This patch introduces a helper sendpage_ok(), it returns true if the checking page, - is not slab page: PageSlab(page) is false. - has page refcount: page_count(page) is not zero All drivers who want to send page to remote end by kernel_sendpage() may use this helper to check whether the page is OK. If the helper does not return true, the driver should try other non sendpage method (e.g. sock_no_sendpage()) to handle the page. Signed-off-by: Coly Li <colyli@suse.de> Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Hannes Reinecke <hare@suse.de> Cc: Jan Kara <jack@suse.com> Cc: Jens Axboe <axboe@kernel.dk> Cc: Mikhail Skorzhinskii <mskorzhinskiy@solarflare.com> Cc: Philipp Reisner <philipp.reisner@linbit.com> Cc: Sagi Grimberg <sagi@grimberg.me> Cc: Vlastimil Babka <vbabka@suse.com> Cc: stable@vger.kernel.org --- v5, include linux/mm.h in include/linux/net.h v4, change sendpage_ok() as an inline helper, and post it as separate patch. v3, introduce a more common sendpage_ok() v2, fix typo in patch subject v1, the initial version. include/linux/net.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)