Re: [PATCH net] virtio-net: fix for skb_over_panic inside big mode

2021-10-09 Thread Xuan Zhuo
On Sat, 9 Oct 2021 05:17:53 -0400, Michael S. Tsirkin  wrote:
> From: Xuan Zhuo 
>
> commit 126285651b7f ("Merge 
> ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net")
> accidentally reverted the effect of
> commit 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode")
> on drivers/net/virtio_net.c
>
> As a result, users of crosvm (which is using large packet mode)
> are experiencing crashes with 5.14-rc1 and above that do not
> occur with 5.13.
>
> Crash trace:
>
> [   61.346677] skbuff: skb_over_panic: text:881ae2c7 len:3762 
> put:3762 head:8a5ec8c22000 data:8a5ec8c22010 tail:0xec2 end:0xec0 
> dev:
> [   61.369192] kernel BUG at net/core/skbuff.c:111!
> [   61.372840] invalid opcode:  [#1] SMP PTI
> [   61.374892] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.14.0-rc1 
> linux-v5.14-rc1-for-mesa-ci.tar.bz2 #1
> [   61.376450] Hardware name: ChromiumOS crosvm, BIOS 0
>
> ..
>
> [   61.393635] Call Trace:
> [   61.394127]  
> [   61.394488]  skb_put.cold+0x10/0x10
> [   61.395095]  page_to_skb+0xf7/0x410
> [   61.395689]  receive_buf+0x81/0x1660
> [   61.396228]  ? netif_receive_skb_list_internal+0x1ad/0x2b0
> [   61.397180]  ? napi_gro_flush+0x97/0xe0
> [   61.397896]  ? detach_buf_split+0x67/0x120
> [   61.398573]  virtnet_poll+0x2cf/0x420
> [   61.399197]  __napi_poll+0x25/0x150
> [   61.399764]  net_rx_action+0x22f/0x280
> [   61.400394]  __do_softirq+0xba/0x257
> [   61.401012]  irq_exit_rcu+0x8e/0xb0
> [   61.401618]  common_interrupt+0x7b/0xa0
> [   61.402270]  
>
> See
> https://lore.kernel.org/r/5edaa2b7c2fe4abd0347b8454b2ac032b6694e2c.camel%40collabora.com
> for the report.
>
> Apply the original 1a8024239da ("virtio-net: fix for skb_over_panic inside 
> big mode")
> again, the original logic still holds:
>
> In virtio-net's large packet mode, there is a hole in the space behind
> buf.
>
> hdr_padded_len - hdr_len
>
> We must take this into account when calculating tailroom.
>
> Cc: Greg KH 
> Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's 
> sufficient tailroom")
> Fixes: 126285651b7f ("Merge 
> ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net")
> Signed-off-by: Xuan Zhuo 
> Reported-by: Corentin Noël 
> Tested-by: Corentin Noël 
> Signed-off-by: Michael S. Tsirkin 

LGTM

Reviewed-by: Xuan Zhuo 

> ---
>
> David, I think we need this in stable, too.
>
>  drivers/net/virtio_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 096c2ac6b7a6..6b0812f44bbf 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info 
> *vi,
>* add_recvbuf_mergeable() + get_mergeable_buf_len()
>*/
>   truesize = headroom ? PAGE_SIZE : truesize;
> - tailroom = truesize - len - headroom;
> + tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len);
>   buf = p - headroom;
>
>   len -= hdr_len;
> --
> MST
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: virtio-net: kernel panic in virtio_net.c

2021-10-09 Thread Xuan Zhuo
On Sat, 9 Oct 2021 07:19:39 +0200, Greg KH  wrote:
> On Sat, Oct 09, 2021 at 12:27:08AM +0800, Xuan Zhuo wrote:
> > On Fri, 8 Oct 2021 10:06:57 +0200, Greg KH  
> > wrote:
> > > On Fri, Oct 08, 2021 at 12:17:26AM +0800, Xuan Zhuo wrote:
> > > > On Thu, 7 Oct 2021 17:25:02 +0200, Greg KH  
> > > > wrote:
> > > > > On Thu, Oct 07, 2021 at 11:06:12PM +0800, Xuan Zhuo wrote:
> > > > > > On Thu, 07 Oct 2021 14:04:22 +0200, Corentin Noël 
> > > > > >  wrote:
> > > > > > > I've been experiencing crashes with 5.14-rc1 and above that do not
> > > > > > > occur with 5.13,
> > > > > >
> > > > > > I should have fixed this problem before. I don't know why, I just 
> > > > > > looked at the
> > > > > > latest net code, and this commit seems to be lost.
> > > > > >
> > > > > >  1a8024239dacf53fcf39c0f07fbf2712af22864f virtio-net: fix for 
> > > > > > skb_over_panic inside big mode
> > > > > >
> > > > > > Can you test this patch again?
> > > > >
> > > > > That commit showed up in 5.13-rc5, so 5.14-rc1 and 5.13 should have 
> > > > > had
> > > > > it in it, right?
> > > > >
> > > >
> > > > Yes, it may be lost due to conflicts during a certain merge.
> > >
> > > Really?  I tried to apply that again to 5.14 and it did not work.  So I
> > > do not understand what to do here, can you try to explain it better?
> >
> > I took a look, and there is actually another missing patch:
> >
> > A. 8fb7da9e990793299c89ed7a4281c235bfdd31f8 virtio_net: get build_skb() buf 
> > by data ptr
> > B. 1a8024239dacf53fcf39c0f07fbf2712af22864f virtio-net: fix for 
> > skb_over_panic inside big mode
> >
> > A is replaced by another patch:
> >
> > commit c32325b8fdf2f979befb9fd5587918c0d5412db3
> > Author: Jakub Kicinski 
> > Date:   Mon Aug 2 10:57:29 2021 -0700
> >
> > virtio-net: realign page_to_skb() after merges
> >
> > We ended up merging two versions of the same patch set:
> >
> > commit 8fb7da9e9907 ("virtio_net: get build_skb() buf by data ptr")
> > commit 5c37711d9f27 ("virtio-net: fix for unable to handle page 
> > fault for address")
> >
> > into net, and
> >
> > commit 7bf64460e3b2 ("virtio-net: get build_skb() buf by data ptr")
> > commit 6c66c147b9a4 ("virtio-net: fix for unable to handle page 
> > fault for address")
> >
> > into net-next. Redo the merge from commit 126285651b7f ("Merge
> > ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net"), so that
> > the most recent code remains.
> >
> > Acked-by: Michael S. Tsirkin 
> > Signed-off-by: Jakub Kicinski 
> > Acked-by: Jason Wang 
> > Signed-off-by: David S. Miller 
> >
> > So after this patch, patch B can be applied normally.
> >
> > So on the latest net branch, only lost
> >
> >   1a8024239dacf53fcf39c0f07fbf2712af22864f virtio-net: fix for 
> > skb_over_panic inside big mode
>
> Again, I do not know what to do here, can you submit the needed fix to
> the networking developers so this gets fixed?

Michael has already submitted the patch.

https://lore.kernel.org/netdev/20211009091604.84141-1-...@redhat.com/T/#u

Thanks.

>
> thanks,
>
> greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()

2021-10-09 Thread Greg KH
On Sat, Oct 09, 2021 at 07:48:28PM +0800, Xianting Tian wrote:
> --- a/drivers/tty/hvc/hvc_console.h
> +++ b/drivers/tty/hvc/hvc_console.h
> @@ -32,13 +32,21 @@
>   */
>  #define HVC_ALLOC_TTY_ADAPTERS   8
>  
> +/*
> + * These sizes are most efficient for vio, because they are the
> + * native transfer size. We could make them selectable in the
> + * future to better deal with backends that want other buffer sizes.
> + */
> +#define N_OUTBUF 16
> +#define N_INBUF  16
> +
> +#define __ALIGNED__ __attribute__((__aligned__(sizeof(long

Does this conflict with what is in hvcs.c?

> +
>  struct hvc_struct {
>   struct tty_port port;
>   spinlock_t lock;
>   int index;
>   int do_wakeup;
> - char *outbuf;
> - int outbuf_size;
>   int n_outbuf;
>   uint32_t vtermno;
>   const struct hv_ops *ops;
> @@ -48,6 +56,18 @@ struct hvc_struct {
>   struct work_struct tty_resize;
>   struct list_head next;
>   unsigned long flags;
> +
> + /* the buf is used in hvc console api for putting chars */
> + char cons_outbuf[N_OUTBUF] __ALIGNED__;
> + spinlock_t cons_outbuf_lock;

Did you look at the placement using pahole as to how this structure now
looks?

> +
> + /* the buf is for putting single char to tty */
> + char outchar;
> + spinlock_t outchar_lock;

So you have a lock for a character and a different one for a longer
string?  Why can they not just use the same lock?  Why are 2 needed at
all, can't you just use the first character of cons_outbuf[] instead?
Surely you do not have 2 sends happening at the same time, right?

> +
> + /* the buf is for putting chars to tty */
> + int outbuf_size;
> + char outbuf[0] __ALIGNED__;

I thought we were not allowing [0] anymore in kernel structures?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()

2021-10-09 Thread Greg KH
On Sat, Oct 09, 2021 at 07:48:28PM +0800, Xianting Tian wrote:
> As well known, hvc backend can register its opertions to hvc backend.
> the operations contain put_chars(), get_chars() and so on.
> 
> Some hvc backend may do dma in its operations. eg, put_chars() of
> virtio-console. But in the code of hvc framework, it may pass DMA
> incapable memory to put_chars() under a specific configuration, which
> is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
> 1, c[] is on stack,
>hvc_console_print():
>   char c[N_OUTBUF] __ALIGNED__;
>   cons_ops[index]->put_chars(vtermnos[index], c, i);
> 2, ch is on stack,
>static void hvc_poll_put_char(,,char ch)
>{
>   struct tty_struct *tty = driver->ttys[0];
>   struct hvc_struct *hp = tty->driver_data;
>   int n;
> 
>   do {
>   n = hp->ops->put_chars(hp->vtermno, , 1);
>   } while (n <= 0);
>}
> 
> Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
> is passed to virtio-console by hvc framework in above code. But I think
> the fix is aggressive, it directly uses kmemdup() to alloc new buffer
> from kmalloc area and do memcpy no matter the memory is in kmalloc area
> or not. But most importantly, it should better be fixed in the hvc
> framework, by changing it to never pass stack memory to the put_chars()
> function in the first place. Otherwise, we still face the same issue if
> a new hvc backend using dma added in the furture.
> 
> In this patch, add 'char cons_outbuf[]' as part of 'struct hvc_struct',
> so hp->cons_outbuf is no longer the stack memory, we can use it in above
> case 1. Add 'char outchar' as part of 'struct hvc_struct', we can use it
> in above case 2. We also add lock for each above buf to protect them
> separately instead of using the global lock of hvc.
> 
> Introduce another array(cons_hvcs[]) for hvc pointers next to the
> cons_ops[] and vtermnos[] arrays. With the array, we can easily find
> hvc's cons_outbuf and its lock.
> 
> With the patch, we can revert the fix c4baad5029.
> 
> Signed-off-by: Xianting Tian 
> Signed-off-by: Shile Zhang 
> ---
>  drivers/tty/hvc/hvc_console.c | 37 +--
>  drivers/tty/hvc/hvc_console.h | 24 +--
>  2 files changed, 44 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index 5bb8c4e44..4d8f112f2 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -41,16 +41,6 @@
>   */
>  #define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
>  
> -/*
> - * These sizes are most efficient for vio, because they are the
> - * native transfer size. We could make them selectable in the
> - * future to better deal with backends that want other buffer sizes.
> - */
> -#define N_OUTBUF 16
> -#define N_INBUF  16
> -
> -#define __ALIGNED__ __attribute__((__aligned__(sizeof(long
> -

Are you sure this applies on top of patch 1/3 here?

> +/*
> + * These sizes are most efficient for vio, because they are the
> + * native transfer size. We could make them selectable in the
> + * future to better deal with backends that want other buffer sizes.
> + */
> +#define N_OUTBUF 16
> +#define N_INBUF  16
> +
> +#define __ALIGNED__ __attribute__((__aligned__(sizeof(long

Again, are you sure this is correct?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH net] virtio-net: fix for skb_over_panic inside big mode

2021-10-09 Thread Michael S. Tsirkin
From: Xuan Zhuo 

commit 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net")
accidentally reverted the effect of
commit 1a8024239da ("virtio-net: fix for skb_over_panic inside big mode")
on drivers/net/virtio_net.c

As a result, users of crosvm (which is using large packet mode)
are experiencing crashes with 5.14-rc1 and above that do not
occur with 5.13.

Crash trace:

[   61.346677] skbuff: skb_over_panic: text:881ae2c7 len:3762 put:3762 
head:8a5ec8c22000 data:8a5ec8c22010 tail:0xec2 end:0xec0 dev:
[   61.369192] kernel BUG at net/core/skbuff.c:111!
[   61.372840] invalid opcode:  [#1] SMP PTI
[   61.374892] CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.14.0-rc1 
linux-v5.14-rc1-for-mesa-ci.tar.bz2 #1
[   61.376450] Hardware name: ChromiumOS crosvm, BIOS 0

..

[   61.393635] Call Trace:
[   61.394127]  
[   61.394488]  skb_put.cold+0x10/0x10
[   61.395095]  page_to_skb+0xf7/0x410
[   61.395689]  receive_buf+0x81/0x1660
[   61.396228]  ? netif_receive_skb_list_internal+0x1ad/0x2b0
[   61.397180]  ? napi_gro_flush+0x97/0xe0
[   61.397896]  ? detach_buf_split+0x67/0x120
[   61.398573]  virtnet_poll+0x2cf/0x420
[   61.399197]  __napi_poll+0x25/0x150
[   61.399764]  net_rx_action+0x22f/0x280
[   61.400394]  __do_softirq+0xba/0x257
[   61.401012]  irq_exit_rcu+0x8e/0xb0
[   61.401618]  common_interrupt+0x7b/0xa0
[   61.402270]  

See
https://lore.kernel.org/r/5edaa2b7c2fe4abd0347b8454b2ac032b6694e2c.camel%40collabora.com
for the report.

Apply the original 1a8024239da ("virtio-net: fix for skb_over_panic inside big 
mode")
again, the original logic still holds:

In virtio-net's large packet mode, there is a hole in the space behind
buf.

hdr_padded_len - hdr_len

We must take this into account when calculating tailroom.

Cc: Greg KH 
Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's 
sufficient tailroom")
Fixes: 126285651b7f ("Merge ra.kernel.org:/pub/scm/linux/kernel/git/netdev/net")
Signed-off-by: Xuan Zhuo 
Reported-by: Corentin Noël 
Tested-by: Corentin Noël 
Signed-off-by: Michael S. Tsirkin 
---

David, I think we need this in stable, too.

 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 096c2ac6b7a6..6b0812f44bbf 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -406,7 +406,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 * add_recvbuf_mergeable() + get_mergeable_buf_len()
 */
truesize = headroom ? PAGE_SIZE : truesize;
-   tailroom = truesize - len - headroom;
+   tailroom = truesize - len - headroom - (hdr_padded_len - hdr_len);
buf = p - headroom;
 
len -= hdr_len;
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 12/16] PCI: Add pci_iomap_host_shared(), pci_iomap_host_shared_range()

2021-10-09 Thread Michael S. Tsirkin
On Fri, Oct 08, 2021 at 05:37:07PM -0700, Kuppuswamy Sathyanarayanan wrote:
> From: Andi Kleen 
> 
> For Confidential VM guests like TDX, the host is untrusted and hence
> the devices emulated by the host or any data coming from the host
> cannot be trusted. So the drivers that interact with the outside world
> have to be hardened by sharing memory with host on need basis
> with proper hardening fixes.
> 
> For the PCI driver case, to share the memory with the host add
> pci_iomap_host_shared() and pci_iomap_host_shared_range() APIs.
> 
> Signed-off-by: Andi Kleen 
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 

So I proposed to make all pci mappings shared, eliminating the need
to patch drivers.

To which Andi replied
One problem with removing the ioremap opt-in is that
it's still possible for drivers to get at devices without going through 
probe.

To which Greg replied:
https://lore.kernel.org/all/yvxbnj431yiww...@kroah.com/
If there are in-kernel PCI drivers that do not do this, they need to be
fixed today.

Can you guys resolve the differences here?

And once they are resolved, mention this in the commit log so
I don't get to re-read the series just to find out nothing
changed in this respect?

I frankly do not believe we are anywhere near being able to harden
an arbitrary kernel config against attack.
How about creating a defconfig that makes sense for TDX then?
Anyone deviating from that better know what they are doing,
this API tweaking is just putting policy into the kernel  ...

> ---
>  Changes since v4:
>  * Replaced "_shared" with "_host_shared" in pci_iomap* APIs
>  * Fixed commit log as per review comments.
> 
>  include/asm-generic/pci_iomap.h |  6 +
>  lib/pci_iomap.c | 47 +
>  2 files changed, 53 insertions(+)
> 
> diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
> index df636c6d8e6c..a4a83c8ab3cf 100644
> --- a/include/asm-generic/pci_iomap.h
> +++ b/include/asm-generic/pci_iomap.h
> @@ -18,6 +18,12 @@ extern void __iomem *pci_iomap_range(struct pci_dev *dev, 
> int bar,
>  extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
>   unsigned long offset,
>   unsigned long maxlen);
> +extern void __iomem *pci_iomap_host_shared(struct pci_dev *dev, int bar,
> +unsigned long max);
> +extern void __iomem *pci_iomap_host_shared_range(struct pci_dev *dev, int 
> bar,
> +  unsigned long offset,
> +  unsigned long maxlen);
> +
>  /* Create a virtual mapping cookie for a port on a given PCI device.
>   * Do not call this directly, it exists to make it easier for architectures
>   * to override */
> diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
> index 57bd92f599ee..2816dc8715da 100644
> --- a/lib/pci_iomap.c
> +++ b/lib/pci_iomap.c
> @@ -25,6 +25,11 @@ static void __iomem *map_ioremap_wc(phys_addr_t addr, 
> size_t size)
>   return ioremap_wc(addr, size);
>  }
>  
> +static void __iomem *map_ioremap_host_shared(phys_addr_t addr, size_t size)
> +{
> + return ioremap_host_shared(addr, size);
> +}
> +
>  static void __iomem *pci_iomap_range_map(struct pci_dev *dev,
>int bar,
>unsigned long offset,
> @@ -106,6 +111,48 @@ void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
>  }
>  EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
>  
> +/**
> + * pci_iomap_host_shared_range - create a virtual shared mapping cookie
> + *for a PCI BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR number
> + * @offset: map memory at the given offset in BAR
> + * @maxlen: max length of the memory to map
> + *
> + * Remap a pci device's resources shared in a confidential guest.
> + * For more details see pci_iomap_range's documentation.

So how does a driver author know when to use this function, and when to
use the regular pci_iomap_range?  Drivers have no idea whether they are
used in a confidential guest, and which ranges are shared, it's a TDX
thing ...

This documentation should really address it.

> + *
> + * @maxlen specifies the maximum length to map. To get access to
> + * the complete BAR from offset to the end, pass %0 here.
> + */
> +void __iomem *pci_iomap_host_shared_range(struct pci_dev *dev, int bar,
> +   unsigned long offset,
> +   unsigned long maxlen)
> +{
> + return pci_iomap_range_map(dev, bar, offset, maxlen,
> +map_ioremap_host_shared, true);
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_host_shared_range);
> +
> +/**
> + * pci_iomap_host_shared - create a virtual shared mapping cookie for a PCI 
> BAR
> + * @dev: PCI device that owns the BAR
> + * @bar: BAR 

Re: [PATCH v5 16/16] x86/tdx: Add cmdline option to force use of ioremap_host_shared

2021-10-09 Thread Michael S. Tsirkin
On Fri, Oct 08, 2021 at 05:37:11PM -0700, Kuppuswamy Sathyanarayanan wrote:
> + ioremap_force_shared= [X86_64, CCG]
> + Force the kernel to use shared memory mappings which do
> + not use ioremap_host_shared/pcimap_host_shared to opt-in
> + to shared mappings with the host. This feature is mainly
> + used by a confidential guest when enabling new drivers
> + without proper shared memory related changes. Please 
> note
> + that this option might also allow other non explicitly
> + enabled drivers to interact with the host in 
> confidential
> + guest, which could cause other security risks. This 
> option
> + will also cause BIOS data structures to be shared with 
> the
> + host, which might open security holes.
> +
>   io7=[HW] IO7 for Marvel-based Alpha systems
>   See comment before marvel_specify_io7 in
>   arch/alpha/kernel/core_marvel.c.

The connection is quite unfortunate IMHO.
Can't there be an option
that unbreaks drivers *without* opening up security holes by
making BIOS shared?

-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v5 12/16] PCI: Add pci_iomap_host_shared(), pci_iomap_host_shared_range()

2021-10-09 Thread Dan Williams
On Sat, Oct 9, 2021 at 2:53 AM Michael S. Tsirkin  wrote:
>
> On Fri, Oct 08, 2021 at 05:37:07PM -0700, Kuppuswamy Sathyanarayanan wrote:
> > From: Andi Kleen 
> >
> > For Confidential VM guests like TDX, the host is untrusted and hence
> > the devices emulated by the host or any data coming from the host
> > cannot be trusted. So the drivers that interact with the outside world
> > have to be hardened by sharing memory with host on need basis
> > with proper hardening fixes.
> >
> > For the PCI driver case, to share the memory with the host add
> > pci_iomap_host_shared() and pci_iomap_host_shared_range() APIs.
> >
> > Signed-off-by: Andi Kleen 
> > Signed-off-by: Kuppuswamy Sathyanarayanan 
> > 
>
> So I proposed to make all pci mappings shared, eliminating the need
> to patch drivers.
>
> To which Andi replied
> One problem with removing the ioremap opt-in is that
> it's still possible for drivers to get at devices without going 
> through probe.
>
> To which Greg replied:
> https://lore.kernel.org/all/yvxbnj431yiww...@kroah.com/
> If there are in-kernel PCI drivers that do not do this, they need to 
> be
> fixed today.
>
> Can you guys resolve the differences here?

I agree with you and Greg here. If a driver is accessing hardware
resources outside of the bind lifetime of one of the devices it
supports, and in a way that neither modrobe-policy nor
device-authorization -policy infrastructure can block, that sounds
like a bug report. Fix those drivers instead of sprinkling
ioremap_shared in select places and with unclear rules about when a
driver is allowed to do "shared" mappings. Let the new
device-authorization mechanism (with policy in userspace) be the
central place where all of these driver "trust" issues are managed.

> And once they are resolved, mention this in the commit log so
> I don't get to re-read the series just to find out nothing
> changed in this respect?
>
> I frankly do not believe we are anywhere near being able to harden
> an arbitrary kernel config against attack.
> How about creating a defconfig that makes sense for TDX then?
> Anyone deviating from that better know what they are doing,
> this API tweaking is just putting policy into the kernel  ...

Right, userspace authorization policy and select driver fixups seems
to be the answer to the raised concerns.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()

2021-10-09 Thread Greg KH
On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
> 
> 在 2021/10/9 下午7:58, Greg KH 写道:
> > Did you look at the placement using pahole as to how this structure now
> > looks?
> 
> thanks for all your commnts. for this one, do you mean I need to remove the
> blank line?  thanks
>

No, I mean to use the tool 'pahole' to see the structure layout that you
just created and determine if it really is the best way to add these new
fields, especially as you are adding huge buffers with odd alignment.

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization