| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120426204450.GA7392@phenom.dumpdata.com>
Date: Thu, 26 Apr 2012 16:44:50 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Ian Campbell <ian.campbell@...rix.com>
Cc: netdev@...r.kernel.org, Wei Liu <wei.liu2@...rix.com>,
Eric Dumazet <eric.dumazet@...il.com>,
"Michael S. Tsirkin" <mst@...hat.com>, xen-devel@...ts.xen.org,
Michał Mirosław <mirq-linux@...e.qmqm.pl>,
David Miller <davem@...emloft.net>
Subject: Re: [Xen-devel] [PATCH 06/10] net: add support for
per-paged-fragment destructors
On Tue, Apr 10, 2012 at 03:26:20PM +0100, Ian Campbell wrote:
> Entities which care about the complete lifecycle of pages which they inject
> into the network stack via an skb paged fragment can choose to set this
> destructor in order to receive a callback when the stack is really finished
> with a page (including all clones, retransmits, pull-ups etc etc).
>
> This destructor will always be propagated alongside the struct page when
> copying skb_frag_t->page. This is the reason I chose to embed the destructor in
> a "struct { } page" within the skb_frag_t, rather than as a separate field,
> since it allows existing code which propagates ->frags[N].page to Just
> Work(tm).
>
> When the destructor is present the page reference counting is done slightly
> differently. No references are held by the network stack on the struct page (it
> is up to the caller to manage this as necessary) instead the network stack will
> track references via the count embedded in the destructor structure. When this
> reference count reaches zero then the destructor will be called and the caller
> can take the necesary steps to release the page (i.e. release the struct page
> reference itself).
>
> The intention is that callers can use this callback to delay completion to
> _their_ callers until the network stack has completely released the page, in
> order to prevent use-after-free or modification of data pages which are still
> in use by the stack.
>
> It is allowable (indeed expected) for a caller to share a single destructor
> instance between multiple pages injected into the stack e.g. a group of pages
> included in a single higher level operation might share a destructor which is
> used to complete that higher level operation.
>
> With this change and the previous two changes to shinfo alignment and field
> orderring it is now the case tyhat on a 64 bit system with 64 byte cache lines,
^^^^ - that.
> everything from nr_frags until the end of frags[0] is on the same cacheline.
>
> Signed-off-by: Ian Campbell <ian.campbell@...rix.com>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <eric.dumazet@...il.com>
> Cc: "Michał Mirosław" <mirq-linux@...e.qmqm.pl>
> Cc: netdev@...r.kernel.org
> ---
> include/linux/skbuff.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> net/core/skbuff.c | 17 +++++++++++++++++
> 2 files changed, 60 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index f0ae39c..6ac283e 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -166,9 +166,15 @@ struct sk_buff;
>
> typedef struct skb_frag_struct skb_frag_t;
>
> +struct skb_frag_destructor {
> + atomic_t ref;
> + int (*destroy)(struct skb_frag_destructor *destructor);
> +};
> +
> struct skb_frag_struct {
> struct {
> struct page *p;
> + struct skb_frag_destructor *destructor;
> } page;
> #if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
> __u32 page_offset;
> @@ -1221,6 +1227,31 @@ static inline int skb_pagelen(const struct sk_buff *skb)
> }
>
> /**
> + * skb_frag_set_destructor - set destructor for a paged fragment
> + * @skb: buffer containing fragment to be initialised
> + * @i: paged fragment index to initialise
> + * @destroy: the destructor to use for this fragment
> + *
> + * Sets @destroy as the destructor to be called when all references to
> + * the frag @i in @skb (tracked over skb_clone, retransmit, pull-ups,
> + * etc) are released.
> + *
> + * When a destructor is set then reference counting is performed on
> + * @destroy->ref. When the ref reaches zero then @destroy->destroy
> + * will be called. The caller is responsible for holding and managing
> + * any other references (such a the struct page reference count).
> + *
> + * This function must be called before any use of skb_frag_ref() or
> + * skb_frag_unref().
> + */
> +static inline void skb_frag_set_destructor(struct sk_buff *skb, int i,
> + struct skb_frag_destructor *destroy)
> +{
> + skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
> + frag->page.destructor = destroy;
> +}
> +
> +/**
> * __skb_fill_page_desc - initialise a paged fragment in an skb
> * @skb: buffer containing fragment to be initialised
> * @i: paged fragment index to initialise
> @@ -1239,6 +1270,7 @@ static inline void __skb_fill_page_desc(struct sk_buff *skb, int i,
> skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> frag->page.p = page;
> + frag->page.destructor = NULL;
> frag->page_offset = off;
> skb_frag_size_set(frag, size);
> }
> @@ -1743,6 +1775,9 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
> return frag->page.p;
> }
>
> +extern void skb_frag_destructor_ref(struct skb_frag_destructor *destroy);
> +extern void skb_frag_destructor_unref(struct skb_frag_destructor *destroy);
> +
> /**
> * __skb_frag_ref - take an addition reference on a paged fragment.
> * @frag: the paged fragment
> @@ -1751,6 +1786,10 @@ static inline struct page *skb_frag_page(const skb_frag_t *frag)
> */
> static inline void __skb_frag_ref(skb_frag_t *frag)
> {
> + if (unlikely(frag->page.destructor)) {
> + skb_frag_destructor_ref(frag->page.destructor);
> + return;
> + }
> get_page(skb_frag_page(frag));
> }
>
> @@ -1774,6 +1813,10 @@ static inline void skb_frag_ref(struct sk_buff *skb, int f)
> */
> static inline void __skb_frag_unref(skb_frag_t *frag)
> {
> + if (unlikely(frag->page.destructor)) {
> + skb_frag_destructor_unref(frag->page.destructor);
> + return;
> + }
> put_page(skb_frag_page(frag));
> }
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index b8a41d6..9ec88ce 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -349,6 +349,23 @@ struct sk_buff *dev_alloc_skb(unsigned int length)
> }
> EXPORT_SYMBOL(dev_alloc_skb);
>
> +void skb_frag_destructor_ref(struct skb_frag_destructor *destroy)
> +{
> + BUG_ON(destroy == NULL);
> + atomic_inc(&destroy->ref);
> +}
> +EXPORT_SYMBOL(skb_frag_destructor_ref);
> +
> +void skb_frag_destructor_unref(struct skb_frag_destructor *destroy)
> +{
> + if (destroy == NULL)
> + return;
> +
> + if (atomic_dec_and_test(&destroy->ref))
> + destroy->destroy(destroy);
> +}
> +EXPORT_SYMBOL(skb_frag_destructor_unref);
> +
> static void skb_drop_list(struct sk_buff **listp)
> {
> struct sk_buff *list = *listp;
> --
> 1.7.2.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@...ts.xen.org
> http://lists.xen.org/xen-devel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists