On Tue, 19 Jan 2021 18:19:40 -0800 Ronak Doshi wrote:
> From: Petr Vandrovec <[email protected]>
>
> vmxnet3: Remove buf_info from device accessible structures
Something happened to the posting, looks like the subject is listed
twice?
> buf_info structures in RX & TX queues are private driver data that
> do not need to be visible to the device. Although there is physical
> address and length in the queue descriptor that points to these
> structures, their layout is not standardized, and device never looks
> at them.
>
> So lets allocate these structures in non-DMA-able memory, and fill
> physical address as all-ones and length as zero in the queue
> descriptor.
>
> That should alleviate worries brought by Martin Radev in
> https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20210104/022829.html
> that malicious vmxnet3 device could subvert SVM/TDX guarantees.
>
> Signed-off-by: Petr Vandrovec <[email protected]>
> Signed-off-by: Ronak Doshi <[email protected]>
> @@ -534,11 +530,13 @@ vmxnet3_tq_create(struct vmxnet3_tx_queue *tq,
> goto err;
> }
>
> - sz = tq->tx_ring.size * sizeof(tq->buf_info[0]);
> - tq->buf_info = dma_alloc_coherent(&adapter->pdev->dev, sz,
> - &tq->buf_info_pa, GFP_KERNEL);
> - if (!tq->buf_info)
> + tq->buf_info = kmalloc_array_node(tq->tx_ring.size, sizeof(tq->buf_info[0]),
> + GFP_KERNEL | __GFP_ZERO,
> + dev_to_node(&adapter->pdev->dev));
kcalloc_node()
> + if (!tq->buf_info) {
> + netdev_err(adapter->netdev, "failed to allocate tx buffer info\n");
Please drop the message, OOM splat will be visible enough. checkpatch
usually points this out
> goto err;
> + }
Same comments for vmxnet3_rq_create()
On 1/21/21, 5:07 PM, "Jakub Kicinski" <[email protected]> wrote:
> On Tue, 19 Jan 2021 18:19:40 -0800 Ronak Doshi wrote:
> > From: Petr Vandrovec <[email protected]>
> >
> > vmxnet3: Remove buf_info from device accessible structures
>
> Something happened to the posting, looks like the subject is listed
> twice?
It got sent twice. Please ignore.
> > - if (!tq->buf_info)
> > + tq->buf_info = kmalloc_array_node(tq->tx_ring.size, sizeof(tq->buf_info[0]),
> > + GFP_KERNEL | __GFP_ZERO,
> > + dev_to_node(&adapter->pdev->dev));
>
> kcalloc_node()
Sure, will use this callback.
> > + if (!tq->buf_info) {
> > + netdev_err(adapter->netdev, "failed to allocate tx buffer info\n")
>
> Please drop the message, OOM splat will be visible enough. checkpatch
> usually points this out
Okay, will drop it. Checkpatch did not complain about the error message though.
Thanks,
Ronak
On Fri, 22 Jan 2021 08:24:59 +0000 Ronak Doshi wrote:
> On 1/21/21, 5:07 PM, "Jakub Kicinski" <[email protected]> wrote:
> > On Tue, 19 Jan 2021 18:19:40 -0800 Ronak Doshi wrote:
> > > + tq->buf_info = kmalloc_array_node(tq->tx_ring.size, sizeof(tq->buf_info[0]),
> > > + GFP_KERNEL | __GFP_ZERO,
> > > + dev_to_node(&adapter->pdev->dev));
> > > + if (!tq->buf_info) {
> > > + netdev_err(adapter->netdev, "failed to allocate tx buffer info\n")
> >
> > Please drop the message, OOM splat will be visible enough. checkpatch
> > usually points this out
>
> Okay, will drop it. Checkpatch did not complain about the error message though.
Looks like it matches alloc_node or alloc_array, but not
alloc_array_node ?
our $allocFunctions = qr{(?x:
(?:(?:devm_)?
(?:kv|k|v)[czm]alloc(?:_node|_array)? |
CCing Joe in case he thinks it's worth fixing
commit 5799b255c491 ("include/linux/slab.h: add kmalloc_array_node() and
kcalloc_node()") was added in 2017. Update the unnecessary OOM message
test to include it.
Signed-off-by: Joe Perches <[email protected]>
Reported-by: Jakub Kicinski <[email protected]>
---
Maybe not worth fixing, but no real effort to fix either.
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4f8494527139..8dbf1986a8de 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -487,7 +487,7 @@ our $logFunctions = qr{(?x:
our $allocFunctions = qr{(?x:
(?:(?:devm_)?
- (?:kv|k|v)[czm]alloc(?:_node|_array)? |
+ (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? |
kstrdup(?:_const)? |
kmemdup(?:_nul)?) |
(?:\w+)?alloc_skb(?:_ip_align)? |