2020-01-17 13:00:58

by Sergey Dyasli

[permalink] [raw]
Subject: [PATCH v2 0/4] basic KASAN support for Xen PV domains

This series allows to boot and run Xen PV kernels (Dom0 and DomU) with
CONFIG_KASAN=y. It has been used internally for some time now with good
results for finding memory corruption issues in Dom0 kernel.

Only Outline instrumentation is supported at the moment.

Sergey Dyasli (2):
kasan: introduce set_pmd_early_shadow()
x86/xen: add basic KASAN support for PV kernel

Ross Lagerwall (2):
xen: teach KASAN about grant tables
xen/netback: fix grant copy across page boundary

arch/x86/mm/kasan_init_64.c | 12 +++++++
arch/x86/xen/Makefile | 7 ++++
arch/x86/xen/enlighten_pv.c | 3 ++
arch/x86/xen/mmu_pv.c | 38 ++++++++++++++++++++
drivers/net/xen-netback/common.h | 2 +-
drivers/net/xen-netback/netback.c | 60 +++++++++++++++++++++++++------
drivers/xen/Makefile | 2 ++
drivers/xen/grant-table.c | 5 ++-
include/xen/xen-ops.h | 10 ++++++
kernel/Makefile | 2 ++
lib/Kconfig.kasan | 3 +-
mm/kasan/init.c | 32 ++++++++++++-----
12 files changed, 154 insertions(+), 22 deletions(-)

--
2.17.1


2020-01-17 13:01:40

by Sergey Dyasli

[permalink] [raw]
Subject: [PATCH v2 4/4] xen/netback: fix grant copy across page boundary

From: Ross Lagerwall <[email protected]>

When KASAN (or SLUB_DEBUG) is turned on, there is a higher chance that
non-power-of-two allocations are not aligned to the next power of 2 of
the size. Therefore, handle grant copies that cross page boundaries.

Signed-off-by: Ross Lagerwall <[email protected]>
Signed-off-by: Sergey Dyasli <[email protected]>
---
v1 --> v2:
- Use sizeof_field(struct sk_buff, cb)) instead of magic number 48
- Slightly update commit message

RFC --> v1:
- Added BUILD_BUG_ON to the netback patch
- xenvif_idx_release() now located outside the loop

CC: Wei Liu <[email protected]>
CC: Paul Durrant <[email protected]>
---
drivers/net/xen-netback/common.h | 2 +-
drivers/net/xen-netback/netback.c | 60 +++++++++++++++++++++++++------
2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb91a1b..e57684415edd 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -155,7 +155,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
grant_handle_t grant_tx_handle[MAX_PENDING_REQS];

- struct gnttab_copy tx_copy_ops[MAX_PENDING_REQS];
+ struct gnttab_copy tx_copy_ops[MAX_PENDING_REQS * 2];
struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
/* passed to gnttab_[un]map_refs with pages under (un)mapping */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 0020b2e8c279..f8774ede9f0e 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -320,6 +320,7 @@ static int xenvif_count_requests(struct xenvif_queue *queue,

struct xenvif_tx_cb {
u16 pending_idx;
+ u8 copies;
};

#define XENVIF_TX_CB(skb) ((struct xenvif_tx_cb *)(skb)->cb)
@@ -439,6 +440,7 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
{
struct gnttab_map_grant_ref *gop_map = *gopp_map;
u16 pending_idx = XENVIF_TX_CB(skb)->pending_idx;
+ u8 copies = XENVIF_TX_CB(skb)->copies;
/* This always points to the shinfo of the skb being checked, which
* could be either the first or the one on the frag_list
*/
@@ -450,23 +452,26 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
int nr_frags = shinfo->nr_frags;
const bool sharedslot = nr_frags &&
frag_get_pending_idx(&shinfo->frags[0]) == pending_idx;
- int i, err;
+ int i, err = 0;

- /* Check status of header. */
- err = (*gopp_copy)->status;
- if (unlikely(err)) {
- if (net_ratelimit())
- netdev_dbg(queue->vif->dev,
+ while (copies) {
+ /* Check status of header. */
+ int newerr = (*gopp_copy)->status;
+ if (unlikely(newerr)) {
+ if (net_ratelimit())
+ netdev_dbg(queue->vif->dev,
"Grant copy of header failed! status: %d pending_idx: %u ref: %u\n",
(*gopp_copy)->status,
pending_idx,
(*gopp_copy)->source.u.ref);
- /* The first frag might still have this slot mapped */
- if (!sharedslot)
- xenvif_idx_release(queue, pending_idx,
- XEN_NETIF_RSP_ERROR);
+ err = newerr;
+ }
+ (*gopp_copy)++;
+ copies--;
}
- (*gopp_copy)++;
+ /* The first frag might still have this slot mapped */
+ if (unlikely(err) && !sharedslot)
+ xenvif_idx_release(queue, pending_idx, XEN_NETIF_RSP_ERROR);

check_frags:
for (i = 0; i < nr_frags; i++, gop_map++) {
@@ -910,6 +915,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
xenvif_tx_err(queue, &txreq, extra_count, idx);
break;
}
+ XENVIF_TX_CB(skb)->copies = 0;

skb_shinfo(skb)->nr_frags = ret;
if (data_len < txreq.size)
@@ -933,6 +939,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
"Can't allocate the frag_list skb.\n");
break;
}
+ XENVIF_TX_CB(nskb)->copies = 0;
}

if (extras[XEN_NETIF_EXTRA_TYPE_GSO - 1].type) {
@@ -990,6 +997,31 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,

queue->tx_copy_ops[*copy_ops].len = data_len;
queue->tx_copy_ops[*copy_ops].flags = GNTCOPY_source_gref;
+ XENVIF_TX_CB(skb)->copies++;
+
+ if (offset_in_page(skb->data) + data_len > XEN_PAGE_SIZE) {
+ unsigned int extra_len = offset_in_page(skb->data) +
+ data_len - XEN_PAGE_SIZE;
+
+ queue->tx_copy_ops[*copy_ops].len -= extra_len;
+ (*copy_ops)++;
+
+ queue->tx_copy_ops[*copy_ops].source.u.ref = txreq.gref;
+ queue->tx_copy_ops[*copy_ops].source.domid =
+ queue->vif->domid;
+ queue->tx_copy_ops[*copy_ops].source.offset =
+ txreq.offset + data_len - extra_len;
+
+ queue->tx_copy_ops[*copy_ops].dest.u.gmfn =
+ virt_to_gfn(skb->data + data_len - extra_len);
+ queue->tx_copy_ops[*copy_ops].dest.domid = DOMID_SELF;
+ queue->tx_copy_ops[*copy_ops].dest.offset = 0;
+
+ queue->tx_copy_ops[*copy_ops].len = extra_len;
+ queue->tx_copy_ops[*copy_ops].flags = GNTCOPY_source_gref;
+
+ XENVIF_TX_CB(skb)->copies++;
+ }

(*copy_ops)++;

@@ -1674,5 +1706,11 @@ static void __exit netback_fini(void)
}
module_exit(netback_fini);

+static void __init __maybe_unused build_assertions(void)
+{
+ BUILD_BUG_ON(sizeof(struct xenvif_tx_cb) >
+ sizeof_field(struct sk_buff, cb));
+}
+
MODULE_LICENSE("Dual BSD/GPL");
MODULE_ALIAS("xen-backend:vif");
--
2.17.1

2020-01-20 09:00:17

by Paul Durrant

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] xen/netback: fix grant copy across page boundary

On Fri, 17 Jan 2020 at 12:59, Sergey Dyasli <[email protected]> wrote:
>
> From: Ross Lagerwall <[email protected]>
>
> When KASAN (or SLUB_DEBUG) is turned on, there is a higher chance that
> non-power-of-two allocations are not aligned to the next power of 2 of
> the size. Therefore, handle grant copies that cross page boundaries.
>
> Signed-off-by: Ross Lagerwall <[email protected]>
> Signed-off-by: Sergey Dyasli <[email protected]>
> ---
> v1 --> v2:
> - Use sizeof_field(struct sk_buff, cb)) instead of magic number 48
> - Slightly update commit message
>
> RFC --> v1:
> - Added BUILD_BUG_ON to the netback patch
> - xenvif_idx_release() now located outside the loop
>
> CC: Wei Liu <[email protected]>
> CC: Paul Durrant <[email protected]>

Acked-by: Paul Durrant <[email protected]>

2020-01-22 10:09:13

by Sergey Dyasli

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] xen/netback: fix grant copy across page boundary

On 20/01/2020 08:58, Paul Durrant wrote:
> On Fri, 17 Jan 2020 at 12:59, Sergey Dyasli <[email protected]> wrote:
>>
>> From: Ross Lagerwall <[email protected]>
>>
>> When KASAN (or SLUB_DEBUG) is turned on, there is a higher chance that
>> non-power-of-two allocations are not aligned to the next power of 2 of
>> the size. Therefore, handle grant copies that cross page boundaries.
>>
>> Signed-off-by: Ross Lagerwall <[email protected]>
>> Signed-off-by: Sergey Dyasli <[email protected]>
>> ---
>> v1 --> v2:
>> - Use sizeof_field(struct sk_buff, cb)) instead of magic number 48
>> - Slightly update commit message
>>
>> RFC --> v1:
>> - Added BUILD_BUG_ON to the netback patch
>> - xenvif_idx_release() now located outside the loop
>>
>> CC: Wei Liu <[email protected]>
>> CC: Paul Durrant <[email protected]>
>
> Acked-by: Paul Durrant <[email protected]>

Thanks! I believe this patch can go in independently from the other
patches in the series. What else is required for this?

--
Sergey

2020-01-22 14:07:02

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] xen/netback: fix grant copy across page boundary

On Wed, Jan 22, 2020 at 10:07:35AM +0000, Sergey Dyasli wrote:
> On 20/01/2020 08:58, Paul Durrant wrote:
> > On Fri, 17 Jan 2020 at 12:59, Sergey Dyasli <[email protected]> wrote:
> >>
> >> From: Ross Lagerwall <[email protected]>
> >>
> >> When KASAN (or SLUB_DEBUG) is turned on, there is a higher chance that
> >> non-power-of-two allocations are not aligned to the next power of 2 of
> >> the size. Therefore, handle grant copies that cross page boundaries.
> >>
> >> Signed-off-by: Ross Lagerwall <[email protected]>
> >> Signed-off-by: Sergey Dyasli <[email protected]>
> >> ---
> >> v1 --> v2:
> >> - Use sizeof_field(struct sk_buff, cb)) instead of magic number 48
> >> - Slightly update commit message
> >>
> >> RFC --> v1:
> >> - Added BUILD_BUG_ON to the netback patch
> >> - xenvif_idx_release() now located outside the loop
> >>
> >> CC: Wei Liu <[email protected]>
> >> CC: Paul Durrant <[email protected]>
> >
> > Acked-by: Paul Durrant <[email protected]>
>
> Thanks! I believe this patch can go in independently from the other
> patches in the series. What else is required for this?

This patch didn't Cc the network development list so David Miller
wouldn't be able to pick it up.

Wei.

>
> --
> Sergey