[bug report] vdpa/mlx5: Add RX counters to debugfs
Hello Eli Cohen,
The patch 7fc5e9ed0777: "vdpa/mlx5: Add RX counters to debugfs" from
Nov 14, 2022, leads to the following Smatch static checker warning:
drivers/vdpa/mlx5/net/mlx5_vnet.c:1497 mlx5_vdpa_add_mac_vlan_rules()
error: uninitialized symbol 'rule'.
drivers/vdpa/mlx5/net/mlx5_vnet.c
1452 static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8
*mac,
1453 struct macvlan_node *node)
1454 {
1455 struct mlx5_flow_destination dests[NUM_DESTS] = {};
1456 struct mlx5_flow_act flow_act = {};
1457 struct mlx5_flow_handle *rule;
1458 struct mlx5_flow_spec *spec;
1459 void *headers_c;
1460 void *headers_v;
1461 u8 *dmac_c;
1462 u8 *dmac_v;
1463 int err;
1464 u16 vid;
1465
1466 spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
1467 if (!spec)
1468 return -ENOMEM;
1469
1470 vid = key2vid(node->macvlan);
1471 spec->match_criteria_enable = MLX5_MATCH_OUTER_HEADERS;
1472 headers_c = MLX5_ADDR_OF(fte_match_param,
spec->match_criteria, outer_headers);
1473 headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value,
outer_headers);
1474 dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c,
outer_headers.dmac_47_16);
1475 dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v,
outer_headers.dmac_47_16);
1476 eth_broadcast_addr(dmac_c);
1477 ether_addr_copy(dmac_v, mac);
1478 if (ndev->mvdev.actual_features &
BIT_ULL(VIRTIO_NET_F_CTRL_VLAN)) {
1479 MLX5_SET(fte_match_set_lyr_2_4, headers_c, cvlan_tag,
1);
1480 MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c,
first_vid);
1481 }
1482 if (node->tagged) {
1483 MLX5_SET(fte_match_set_lyr_2_4, headers_v, cvlan_tag,
1);
1484 MLX5_SET(fte_match_set_lyr_2_4, headers_v, first_vid,
vid);
1485 }
1486 flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
1487 dests[0].type = MLX5_FLOW_DESTINATION_TYPE_TIR;
1488 dests[0].tir_num = ndev->res.tirn;
1489 err = add_steering_counters(ndev, node, _act, dests);
1490 if (err)
1491 goto out_free;
1492
1493 #if defined(CONFIG_MLX5_VDPA_STEERING_DEBUG)
1494 dests[1].counter_id = mlx5_fc_id(node->ucast_counter.counter);
1495 #endif
1496 node->ucast_rule = mlx5_add_flow_rules(ndev->rxft, spec,
_act, dests, NUM_DESTS);
--> 1497 if (IS_ERR(rule)) {
Checking the wrong variable. It looks like maybe the fix for this
was already posted but only some of the thread made it to lore.kernel.org
so it's impossible to tell for sure.
1498 err = PTR_ERR(rule);
1499 goto err_ucast;
1500 }
1501
1502 #if defined(CONFIG_MLX5_VDPA_STEERING_DEBUG)
1503 dests[1].counter_id = mlx5_fc_id(node->mcast_counter.counter);
1504 #endif
1505
1506 memset(dmac_c, 0, ETH_ALEN);
1507 memset(dmac_v, 0, ETH_ALEN);
1508 dmac_c[0] = 1;
1509 dmac_v[0] = 1;
1510 node->mcast_rule = mlx5_add_flow_rules(ndev->rxft, spec,
_act, dests, NUM_DESTS);
1511 if (IS_ERR(rule)) {
Here too.
1512 err = PTR_ERR(rule);
1513 goto err_mcast;
1514 }
1515 kvfree(spec);
1516 mlx5_vdpa_add_rx_counters(ndev, node);
1517 return 0;
1518
1519 err_mcast:
1520 mlx5_del_flow_rules(node->ucast_rule);
1521 err_ucast:
1522 remove_steering_counters(ndev, node);
1523 out_free:
1524 kvfree(spec);
1525 return err;
1526 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vduse: Validate vq_num in vduse_validate_config()
On Mon, Nov 28, 2022 at 11:53:12AM +0100, Stefano Garzarella wrote: > On Mon, Nov 28, 2022 at 12:36:26AM -0800, Harshit Mogalapalli wrote: > > Add a limit to 'config->vq_num' which is user controlled data which > > comes from an vduse_ioctl to prevent large memory allocations. > > > > This is found using static analysis with smatch. > > > > Suggested-by: Michael S. Tsirkin > > Signed-off-by: Harshit Mogalapalli > > --- > > v1->v2: Change title of the commit and description, add a limit to > > vq_num. > > > > Note: I think here 0x is the max size of vring = no: of vqueues. > > Only compile and boot tested. > > --- > > drivers/vdpa/vdpa_user/vduse_dev.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c > > b/drivers/vdpa/vdpa_user/vduse_dev.c > > index 35dceee3ed56..31017ebc4d7c 100644 > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > @@ -1440,6 +1440,9 @@ static bool vduse_validate_config(struct > > vduse_dev_config *config) > > if (config->config_size > PAGE_SIZE) > > return false; > > > > + if (config->vq_num > 0x) > > What about using U16_MAX here? Where is the ->vq_num stored in a u16? I looked for this but didn't see it. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vduse: Fix a possible warning in vduse_create_dev()
Btw, after you add the check to vduse_validate_config() you can test that it silences the Smatch warning by doing: kchecker --info drivers/vdpa/vdpa_user/vduse_dev.c | tee out ~/smatch/smatch_data/db/reload_partial.sh out kchecker drivers/vdpa/vdpa_user/vduse_dev.c You might need to do a second --info and reload for the changes to propagate. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2] virtio-gpu: fix shift wrapping bug in virtio_gpu_fence_event_create()
The ->ring_idx_mask variable is a u64 so static checkers, Smatch in
this case, complain if the BIT() is not also a u64.
drivers/gpu/drm/virtio/virtgpu_ioctl.c:50 virtio_gpu_fence_event_create()
warn: should '(1 << ring_idx)' be a 64 bit type?
Fixes: cd7f5ca33585 ("drm/virtio: implement context init: add
virtio_gpu_fence_event")
Signed-off-by: Dan Carpenter
---
v2: Style change. Use BIT_ULL().
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 3b1701607aae..5d05093014ac 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -47,7 +47,7 @@ static int virtio_gpu_fence_event_create(struct drm_device
*dev,
struct virtio_gpu_fence_event *e = NULL;
int ret;
- if (!(vfpriv->ring_idx_mask & (1 << ring_idx)))
+ if (!(vfpriv->ring_idx_mask & BIT_ULL(ring_idx)))
return 0;
e = kzalloc(sizeof(*e), GFP_KERNEL);
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio-gpu: fix shift wrapping bug in virtio_gpu_fence_event_create()
On Thu, Sep 15, 2022 at 05:45:46PM -0700, Chia-I Wu wrote:
> On Thu, Sep 15, 2022 at 4:14 AM Dan Carpenter
> wrote:
> >
> > The ->ring_idx_mask variable is a u64 so static checkers, Smatch in
> > this case, complain if the BIT() is not also a u64.
> >
> > drivers/gpu/drm/virtio/virtgpu_ioctl.c:50 virtio_gpu_fence_event_create()
> > warn: should '(1 << ring_idx)' be a 64 bit type?
> >
> > Fixes: cd7f5ca33585 ("drm/virtio: implement context init: add
> > virtio_gpu_fence_event")
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > index 3b1701607aae..14eedb75f8a8 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> > @@ -47,7 +47,7 @@ static int virtio_gpu_fence_event_create(struct
> > drm_device *dev,
> > struct virtio_gpu_fence_event *e = NULL;
> > int ret;
> >
> > - if (!(vfpriv->ring_idx_mask & (1 << ring_idx)))
> > + if (!(vfpriv->ring_idx_mask & (1ULL << ring_idx)))
> BIT_ULL(ring_indx)?
>
Sure. I can resend.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio-gpu: fix shift wrapping bug in virtio_gpu_fence_event_create()
The ->ring_idx_mask variable is a u64 so static checkers, Smatch in
this case, complain if the BIT() is not also a u64.
drivers/gpu/drm/virtio/virtgpu_ioctl.c:50 virtio_gpu_fence_event_create()
warn: should '(1 << ring_idx)' be a 64 bit type?
Fixes: cd7f5ca33585 ("drm/virtio: implement context init: add
virtio_gpu_fence_event")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 3b1701607aae..14eedb75f8a8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -47,7 +47,7 @@ static int virtio_gpu_fence_event_create(struct drm_device
*dev,
struct virtio_gpu_fence_event *e = NULL;
int ret;
- if (!(vfpriv->ring_idx_mask & (1 << ring_idx)))
+ if (!(vfpriv->ring_idx_mask & (1ULL << ring_idx)))
return 0;
e = kzalloc(sizeof(*e), GFP_KERNEL);
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vduse: prevent uninitialized memory accesses
On Fri, Aug 26, 2022 at 06:16:05PM +0200, Maxime Coquelin wrote: > If the VDUSE application provides a smaller config space > than the driver expects, the driver may use uninitialized > memory from the stack. > > This patch prevents it by initializing the buffer passed by > the driver to store the config value. > > Signed-off-by: Maxime Coquelin This sounds like it needs a Fixes tag? regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 1/4] vdpa: Add suspend operation
On Thu, Aug 11, 2022 at 07:23:44AM -0400, Michael S. Tsirkin wrote: > On Thu, Aug 11, 2022 at 01:15:08PM +0300, Dan Carpenter wrote: > > On Thu, Aug 11, 2022 at 04:27:32AM -0400, Michael S. Tsirkin wrote: > > > On Wed, Aug 10, 2022 at 07:15:09PM +0200, Eugenio Pérez wrote: > > > > This operation is optional: It it's not implemented, backend feature bit > > > > will not be exposed. > > > > > > > > Signed-off-by: Eugenio Pérez > > > > Message-Id: <20220623160738.632852-2-epere...@redhat.com> > > > > Signed-off-by: Michael S. Tsirkin > > > > > > What is this message id doing here? > > > > > > > I like the Message-Id tag. It means you can `b4 mbox ` and get > > the thread. > > Yes it makes sense in git. But I don't see what it does in this patch > posted to the list. Ah. Okay. I misunderstood. > It seems to refer to the previous version of the > patch here. Which is ok I guess but better called out e.g. > > Previous-version: <20220623160738.632852-2-epere...@redhat.com> > Let's not go over board with storing previous threads. That seems like a headache... regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vdpa/mlx5: Support different address spaces for control and data
Hello Eli Cohen,
The patch d5358cd0e369: "vdpa/mlx5: Support different address spaces
for control and data" from Jul 14, 2022, leads to the following
Smatch static checker warning:
drivers/vdpa/mlx5/net/mlx5_vnet.c:2676 mlx5_vdpa_set_map()
error: uninitialized symbol 'err'.
drivers/vdpa/mlx5/net/mlx5_vnet.c
2657 static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int
asid,
2658 struct vhost_iotlb *iotlb)
2659 {
2660 struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
2661 struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
2662 int err;
2663
2664 down_write(>reslock);
2665 if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
2666 err = set_map_data(mvdev, iotlb);
2667 if (err)
2668 goto out;
2669 }
2670
2671 if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
2672 err = set_map_control(mvdev, iotlb);
err not initialized on else path. My guess is that one or both of these
conditions has to be true and this is a false positive but I don't know
the code well enough to be sure.
2673
2674 out:
2675 up_write(>reslock);
--> 2676 return err;
2677 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 1/4] vdpa: Add suspend operation
On Thu, Aug 11, 2022 at 04:27:32AM -0400, Michael S. Tsirkin wrote: > On Wed, Aug 10, 2022 at 07:15:09PM +0200, Eugenio Pérez wrote: > > This operation is optional: It it's not implemented, backend feature bit > > will not be exposed. > > > > Signed-off-by: Eugenio Pérez > > Message-Id: <20220623160738.632852-2-epere...@redhat.com> > > Signed-off-by: Michael S. Tsirkin > > What is this message id doing here? > I like the Message-Id tag. It means you can `b4 mbox ` and get the thread. Linus has complained (rough remembering) that everyone is using the Link: tag for links to the patch itself. It's supposed to be for Links to bugzilla or to the spec or whatever. Extra information, too much to put in the commit message. Now the Link tag is useless because it either points to the patch or to a bugzilla. Depend on what you want it to do, it *always* points to the opposite thing. But I can't remember what people settled on as the alternative to use to link to lore... In theory, we should be able to figure out the link to lore automatically and there have been a couple projects which tried to do this but they can't make it work 100%. Maintainers massage and reformat the patches too much before applying them. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [linux-next:master] BUILD REGRESSION 088b9c375534d905a4d337c78db3b3bfbb52c4a0
On Thu, Jul 07, 2022 at 07:02:58AM -0700, Guenter Roeck wrote: > and the NULL > dereferences in the binder driver are at the very least suspicious. The NULL dereferences in binder are just nonsense Sparse annotations. They don't affect runtime. drivers/android/binder.c:1481:19-23: ERROR: from is NULL but dereferenced. drivers/android/binder.c:2920:29-33: ERROR: target_thread is NULL but dereferenced. drivers/android/binder.c:353:25-35: ERROR: node -> proc is NULL but dereferenced. drivers/android/binder.c:4888:16-20: ERROR: t is NULL but dereferenced. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/2] vdpa/mlx5: clean up indenting in handle_ctrl_vlan()
These lines were supposed to be indented. Signed-off-by: Dan Carpenter --- drivers/vdpa/mlx5/net/mlx5_vnet.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index c964f4161d7f..83607b7488f1 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -1817,10 +1817,10 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct mlx5_vdpa_dev *mvdev, u8 cmd) status = VIRTIO_NET_OK; break; default: - break; -} + break; + } -return status; + return status; } static void mlx5_cvq_kick_handler(struct work_struct *work) -- 2.35.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/2] vdpa/mlx5: fix error code for deleting vlan
Return success if we were able to delete a vlan. The current code
always returns failure.
Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
Signed-off-by: Dan Carpenter
---
>From review. (Not tested).
drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b7a955479156..c964f4161d7f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1814,6 +1814,7 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
id = mlx5vdpa16_to_cpu(mvdev, vlan);
mac_vlan_del(ndev, ndev->config.mac, id, true);
+ status = VIRTIO_NET_OK;
break;
default:
break;
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit
On Mon, May 30, 2022 at 05:27:25PM +0300, Dan Carpenter wrote:
> On Fri, May 27, 2022 at 10:36:55AM +0300, Dan Carpenter wrote:
> > static void match_pointer(struct expression *ret_value)
> > {
> > struct symbol *type;
> > char *name;
> >
> > type = cur_func_return_type();
> > if (!type || sval_type_max(type).value != 1)
> > return;
> >
> > if (!is_pointer(ret_value))
> > return;
> >
> > name = expr_to_str(ret_value);
> > sm_msg("'%s' return pointer cast to bool", name);
> > free_string(name);
> > }
>
> I found a bug in Smatch with this check. With the Smatch bug fixed then
> there is only only place which does a legitimate implied pointer to bool
> cast. A different function returns NULL on error instead of false.
>
> drivers/gpu/drm/i915/display/intel_dmc.c:249 intel_dmc_has_payload()
> 'i915->dmc.dmc_info[0]->payload' return pointer cast to bool
> drivers/net/wireless/rndis_wlan.c:1980 rndis_bss_info_update() '(0)' return
> pointer cast to bool
> drivers/net/wireless/rndis_wlan.c:1989 rndis_bss_info_update() '(0)' return
> pointer cast to bool
> drivers/net/wireless/rndis_wlan.c:1995 rndis_bss_info_update() '(0)' return
> pointer cast to bool
Hm... I found another. I'm not sure why it wasn't showing up before.
lib/assoc_array.c:941 assoc_array_insert_mid_shortcut() 'edit' return pointer
cast to bool
That's weird. I will re-run the test.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit
On Fri, May 27, 2022 at 10:36:55AM +0300, Dan Carpenter wrote:
> static void match_pointer(struct expression *ret_value)
> {
> struct symbol *type;
> char *name;
>
> type = cur_func_return_type();
> if (!type || sval_type_max(type).value != 1)
> return;
>
> if (!is_pointer(ret_value))
> return;
>
> name = expr_to_str(ret_value);
> sm_msg("'%s' return pointer cast to bool", name);
> free_string(name);
> }
I found a bug in Smatch with this check. With the Smatch bug fixed then
there is only only place which does a legitimate implied pointer to bool
cast. A different function returns NULL on error instead of false.
drivers/gpu/drm/i915/display/intel_dmc.c:249 intel_dmc_has_payload()
'i915->dmc.dmc_info[0]->payload' return pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1980 rndis_bss_info_update() '(0)' return
pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1989 rndis_bss_info_update() '(0)' return
pointer cast to bool
drivers/net/wireless/rndis_wlan.c:1995 rndis_bss_info_update() '(0)' return
pointer cast to bool
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit
On Fri, May 27, 2022 at 08:50:16AM +0200, Eugenio Perez Martin wrote:
> On Thu, May 26, 2022 at 9:07 PM Dan Carpenter
> wrote:
> >
> > On Thu, May 26, 2022 at 07:00:06PM +0200, Eugenio Perez Martin wrote:
> > > > It feels like returning any literal that isn't 1 or 0 should trigger a
> > > > warning... I've written that and will check it out tonight.
> > > >
> > >
> > > I'm not sure this should be so strict, or "literal" does not include
> > > pointers?
> > >
> >
> > What I mean in exact terms, is that if you're returning a known value
> > and the function returns bool then the known value should be 0 or 1.
> > Don't "return 3;". This new warning will complain if you return a known
> > pointer as in "return ". It won't complain if you return an
> > unknown pointer "return p;".
> >
>
> Ok, thanks for the clarification.
>
> > > As an experiment, can Smatch be used to count how many times a
> > > returned pointer is converted to int / bool before returning vs not
> > > converted?
> >
> > I'm not super excited to write that code... :/
> >
>
> Sure, I understand. I meant if it was possible or if that is too far
> beyond its scope.
To be honest, I misread what you were asking. GCC won't let you return
a pointer with an implied cast to int. It has to be explicit. So there
are zero of those. It's not hard to look for pointers with an implied
cast to bool.
static void match_pointer(struct expression *ret_value)
{
struct symbol *type;
char *name;
type = cur_func_return_type();
if (!type || sval_type_max(type).value != 1)
return;
if (!is_pointer(ret_value))
return;
name = expr_to_str(ret_value);
sm_msg("'%s' return pointer cast to bool", name);
free_string(name);
}
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit
On Thu, May 26, 2022 at 07:00:06PM +0200, Eugenio Perez Martin wrote: > > It feels like returning any literal that isn't 1 or 0 should trigger a > > warning... I've written that and will check it out tonight. > > > > I'm not sure this should be so strict, or "literal" does not include pointers? > What I mean in exact terms, is that if you're returning a known value and the function returns bool then the known value should be 0 or 1. Don't "return 3;". This new warning will complain if you return a known pointer as in "return ". It won't complain if you return an unknown pointer "return p;". > As an experiment, can Smatch be used to count how many times a > returned pointer is converted to int / bool before returning vs not > converted? I'm not super excited to write that code... :/ > > I find Smatch interesting, especially when switching between projects > frequently. Does it support changing the code like clang-format? To > offload cognitive load to tools is usually good :). No. Coccinelle does that really well though. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 03:28:25PM +0100, Matthew Wilcox wrote: > On Thu, May 26, 2022 at 11:48:32AM +0300, Dan Carpenter wrote: > > On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > > > Bizarre this started showing up now. The recent patch was: > > > > > > - info->alloced += compound_nr(page); > > > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > > > + info->alloced += folio_nr_pages(folio); > > > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > > > > > so it could tell that compound_order() was small, but folio_order() > > > might be large? > > > > The old code also generates a warning on my test system. Smatch thinks > > both compound_order() and folio_order() are 0-255. I guess because of > > the "unsigned char compound_order;" in the struct page. > > It'd be nice if we could annotate that as "contains a value between > 1 and BITS_PER_LONG - PAGE_SHIFT". Then be able to optionally enable > a checker that ensures that's true on loads/stores. Maybe we need a > language that isn't C :-P Ada can do this ... I don't think Rust can. Machine Parsable Comments. It's a matter of figuring out the best format and writing the code. In Smatch, I have table of hard coded return values in the format: https://github.com/error27/smatch/blob/master/smatch_data/db/kernel.return_fixes I don't have code to handle something like BITS_PER_LONG or PAGE_SHIFT. To be honest, Smatch code always assumes that PAGE_SIZE is 4096 but I should actually look it up... It's not impossible to do. The GFP_KERNEL values changed enough so that I eventually made that look up the actual defines. I also have a table in the database where I could edit the values of (struct page)->compound_order. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/4] vhost-vdpa: introduce STOP backend feature bit
On Thu, May 26, 2022 at 02:44:02PM +0200, Eugenio Perez Martin wrote:
> > >> +static bool vhost_vdpa_can_stop(const struct vhost_vdpa *v) {
> > >> + struct vdpa_device *vdpa = v->vdpa;
> > >> + const struct vdpa_config_ops *ops = vdpa->config;
> > >> +
> > >> + return ops->stop;
> > >> [GD>>] Would it be better to explicitly return a bool to match the
> > >> return type?
> > >
> > >I'm not sure about the kernel code style regarding that casting. Maybe
> > >it's better to return !!ops->stop here. The macros likely and unlikely
> > >do that.
> >
> > IIUC `ops->stop` is a function pointer, so what about
> >
> > return ops->stop != NULL;
> >
>
> I'm ok with any method proposed. Both three ways can be found in the
> kernel so I think they are all valid (although the double negation is
> from bool to integer in (0,1) set actually).
>
> Maybe Jason or Michael (as maintainers) can state the preferred method here.
Always just do whatever the person who responded feels like because
they're likely the person who cares the most. ;)
I don't think there are any static analysis tools which will complain
about this. Smatch will complain if you return a negative literal.
It feels like returning any literal that isn't 1 or 0 should trigger a
warning... I've written that and will check it out tonight.
Really anything negative should trigger a warning. See new Smatch stuff
below.
regards,
dan carpenter
TEST CASE =
int x;
_Bool one(int *p)
{
if (p)
x = -2;
return x;
}
_Bool two(int *p)
{
return -4; // returning 2 triggers a warning now
}
=== OUTPUT =
test.c:10 one() warn: potential negative cast to bool 'x'
test.c:14 two() warn: signedness bug returning '(-4)'
test.c:14 two() warn: '(-4)' is not bool
=== CODE ===
#include "smatch.h"
#include "smatch_extra.h"
#include "smatch_slist.h"
static int my_id;
static void match_literals(struct expression *ret_value)
{
struct symbol *type;
sval_t sval;
type = cur_func_return_type();
if (!type || sval_type_max(type).value != 1)
return;
if (!get_implied_value(ret_value, ))
return;
if (sval.value == 0 || sval.value == 1)
return;
sm_warning("'%s' is not bool", sval_to_str(sval));
}
static void match_any_negative(struct expression *ret_value)
{
struct symbol *type;
struct sm_state *extra, *sm;
sval_t sval;
char *name;
type = cur_func_return_type();
if (!type || sval_type_max(type).value != 1)
return;
extra = get_extra_sm_state(ret_value);
if (!extra)
return;
FOR_EACH_PTR(extra->possible, sm) {
if (estate_get_single_value(sm->state, ) &&
sval_is_negative(sval)) {
name = expr_to_str(ret_value);
sm_warning("potential negative cast to bool '%s'",
name);
free_string(name);
return;
}
} END_FOR_EACH_PTR(sm);
}
void check_bool_return(int id)
{
my_id = id;
add_hook(_literals, RETURN_HOOK);
add_hook(_any_negative, RETURN_HOOK);
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Thu, May 26, 2022 at 02:16:34AM +0100, Matthew Wilcox wrote: > Bizarre this started showing up now. The recent patch was: > > - info->alloced += compound_nr(page); > - inode->i_blocks += BLOCKS_PER_PAGE << compound_order(page); > + info->alloced += folio_nr_pages(folio); > + inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > so it could tell that compound_order() was small, but folio_order() > might be large? The old code also generates a warning on my test system. Smatch thinks both compound_order() and folio_order() are 0-255. I guess because of the "unsigned char compound_order;" in the struct page. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [linux-next:master] BUILD REGRESSION 8cb8311e95e3bb58bd84d6350365f14a718faa6d
On Wed, May 25, 2022 at 02:50:56PM -0700, Andrew Morton wrote: > On Thu, 26 May 2022 05:35:20 +0800 kernel test robot wrote: > > > tree/branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > branch HEAD: 8cb8311e95e3bb58bd84d6350365f14a718faa6d Add linux-next > > specific files for 20220525 > > > > Error/Warning reports: > > > > ... > > > > Unverified Error/Warning (likely false positive, please contact us if > > interested): > > Could be so. > > > mm/shmem.c:1948 shmem_getpage_gfp() warn: should '(((1) << 12) / 512) << > > folio_order(folio)' be a 64 bit type? > > I've been seeing this one for a while. And from this report I can't > figure out what tool emitted it. Clang? This is a Smatch warning. I normally look over Smatch warnings before forwarding kbuild-bot emails but this email is a grab bag of static checker warnings from different tools. This warning has a high rate of false positives so I'm going to disable it by default. > > > > > ... > > > > |-- i386-randconfig-m021 > > | `-- > > mm-shmem.c-shmem_getpage_gfp()-warn:should-((()-)-)-folio_order(folio)-be-a-bit-type > > If you're going to use randconfig then shouldn't you make the config > available? Or maybe quote the KCONFIG_SEED - presumably there's a way > for others to regenerate. > > Anyway, the warning seems wrong to me. > > > #define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT) > > #define BLOCKS_PER_PAGE (PAGE_SIZE/512) > > inode->i_blocks += BLOCKS_PER_PAGE << folio_order(folio); > > so the RHS here should have unsigned long type. Being able to generate > the cpp output would be helpful. That requires the .config. The heuristic is that "inode->i_blocks" is a u64 but this .config must be for a 32bit CPU. I'm just going to turn off all these warnings until I can figure out a better heuristic. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vhost-vdpa: return -EFAULT on copy_to_user() failure
The copy_to_user() function returns the number of bytes remaining to be
copied. However, we need to return a negative error code, -EFAULT, to
the user.
Fixes: 87f4c217413a ("vhost-vdpa: introduce uAPI to get the number of virtqueue
groups")
Fixes: e96ef636f154 ("vhost-vdpa: introduce uAPI to get the number of address
spaces")
Signed-off-by: Dan Carpenter
---
drivers/vhost/vdpa.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 3e86080041fc..935a1d0ddb97 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -609,11 +609,13 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
r = vhost_vdpa_get_vring_num(v, argp);
break;
case VHOST_VDPA_GET_GROUP_NUM:
- r = copy_to_user(argp, >vdpa->ngroups,
-sizeof(v->vdpa->ngroups));
+ if (copy_to_user(argp, >vdpa->ngroups,
+sizeof(v->vdpa->ngroups)))
+ r = -EFAULT;
break;
case VHOST_VDPA_GET_AS_NUM:
- r = copy_to_user(argp, >vdpa->nas, sizeof(v->vdpa->nas));
+ if (copy_to_user(argp, >vdpa->nas, sizeof(v->vdpa->nas)))
+ r = -EFAULT;
break;
case VHOST_SET_LOG_BASE:
case VHOST_SET_LOG_FD:
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpasim: Off by one in vdpasim_set_group_asid()
The > comparison needs to be >= to prevent an out of bounds access
of the vdpasim->iommu[] array. The vdpasim->iommu[] is allocated in
vdpasim_create() and it has vdpasim->dev_attr.nas elements.
Fixes: 87e5afeac247 ("vdpasim: control virtqueue support")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 50d721072beb..0f2865899647 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -567,7 +567,7 @@ static int vdpasim_set_group_asid(struct vdpa_device *vdpa,
unsigned int group,
if (group > vdpasim->dev_attr.ngroups)
return -EINVAL;
- if (asid > vdpasim->dev_attr.nas)
+ if (asid >= vdpasim->dev_attr.nas)
return -EINVAL;
iommu = >iommu[asid];
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 2/5] virtio-crypto: use private buffer for control request
Hi zhenwei,
url:
https://github.com/intel-lab-lkp/linux/commits/zhenwei-pi/virtio-crypto-Improve-performance/20220424-184732
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: i386-randconfig-m021
(https://download.01.org/0day-ci/archive/20220424/202204242344.jepumdzp-...@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:165
virtio_crypto_alg_akcipher_init_session() error: potentially dereferencing
uninitialized 'input'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:230
virtio_crypto_alg_akcipher_close_session() error: uninitialized symbol
'vc_ctrl_req'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232
virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing
uninitialized 'ctrl_status'.
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:232
virtio_crypto_alg_akcipher_close_session() error: potentially dereferencing
uninitialized 'destroy_session'.
vim +/input +165 drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
59ca6c93387d32 zhenwei pi 2022-03-02 99 static int
virtio_crypto_alg_akcipher_init_session(struct virtio_crypto_akcipher_ctx *ctx,
59ca6c93387d32 zhenwei pi 2022-03-02 100 struct
virtio_crypto_ctrl_header *header, void *para,
59ca6c93387d32 zhenwei pi 2022-03-02 101 const uint8_t *key,
unsigned int keylen)
59ca6c93387d32 zhenwei pi 2022-03-02 102 {
59ca6c93387d32 zhenwei pi 2022-03-02 103 struct scatterlist outhdr_sg,
key_sg, inhdr_sg, *sgs[3];
59ca6c93387d32 zhenwei pi 2022-03-02 104 struct virtio_crypto *vcrypto =
ctx->vcrypto;
59ca6c93387d32 zhenwei pi 2022-03-02 105 uint8_t *pkey;
59ca6c93387d32 zhenwei pi 2022-03-02 106 unsigned int inlen;
59ca6c93387d32 zhenwei pi 2022-03-02 107 int err;
59ca6c93387d32 zhenwei pi 2022-03-02 108 unsigned int num_out = 0,
num_in = 0;
bb26cab9a7c25d zhenwei pi 2022-04-24 109 struct
virtio_crypto_op_ctrl_req *ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 110 struct
virtio_crypto_session_input *input;
286da9ed04239c zhenwei pi 2022-04-24 111 struct
virtio_crypto_ctrl_request *vc_ctrl_req;
59ca6c93387d32 zhenwei pi 2022-03-02 112
59ca6c93387d32 zhenwei pi 2022-03-02 113 pkey = kmemdup(key, keylen,
GFP_ATOMIC);
59ca6c93387d32 zhenwei pi 2022-03-02 114 if (!pkey)
59ca6c93387d32 zhenwei pi 2022-03-02 115 return -ENOMEM;
59ca6c93387d32 zhenwei pi 2022-03-02 116
286da9ed04239c zhenwei pi 2022-04-24 117 vc_ctrl_req =
kzalloc(sizeof(*vc_ctrl_req), GFP_KERNEL);
286da9ed04239c zhenwei pi 2022-04-24 118 if (!vc_ctrl_req) {
286da9ed04239c zhenwei pi 2022-04-24 119 err = -ENOMEM;
286da9ed04239c zhenwei pi 2022-04-24 120 goto out;
286da9ed04239c zhenwei pi 2022-04-24 121 }
286da9ed04239c zhenwei pi 2022-04-24 122
286da9ed04239c zhenwei pi 2022-04-24 123 ctrl = _ctrl_req->ctrl;
bb26cab9a7c25d zhenwei pi 2022-04-24 124 memcpy(>header, header,
sizeof(ctrl->header));
bb26cab9a7c25d zhenwei pi 2022-04-24 125 memcpy(>u, para,
sizeof(ctrl->u));
286da9ed04239c zhenwei pi 2022-04-24 126 input = _ctrl_req->input;
bb26cab9a7c25d zhenwei pi 2022-04-24 127 input->status =
cpu_to_le32(VIRTIO_CRYPTO_ERR);
59ca6c93387d32 zhenwei pi 2022-03-02 128
bb26cab9a7c25d zhenwei pi 2022-04-24 129 sg_init_one(_sg, ctrl,
sizeof(*ctrl));
59ca6c93387d32 zhenwei pi 2022-03-02 130 sgs[num_out++] = _sg;
59ca6c93387d32 zhenwei pi 2022-03-02 131
59ca6c93387d32 zhenwei pi 2022-03-02 132 sg_init_one(_sg, pkey,
keylen);
59ca6c93387d32 zhenwei pi 2022-03-02 133 sgs[num_out++] = _sg;
59ca6c93387d32 zhenwei pi 2022-03-02 134
bb26cab9a7c25d zhenwei pi 2022-04-24 135 sg_init_one(_sg, input,
sizeof(*input));
59ca6c93387d32 zhenwei pi 2022-03-02 136 sgs[num_out + num_in++] =
_sg;
59ca6c93387d32 zhenwei pi 2022-03-02 137
286da9ed04239c zhenwei pi 2022-04-24 138 spin_lock(>ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 139 err =
virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out, num_in, vcrypto, GFP_ATOMIC);
286da9ed04239c zhenwei pi 2022-04-24 140 if (err < 0) {
286da9ed04239c zhenwei pi 2022-04-24 141
spin_unlock(>ctrl_lock);
59ca6c93387d32 zhenwei pi 2022-03-02 142 goto out;
286da9ed04239c zhenwei pi 2022-04-24 143 }
59ca6c93387d32 zhenwei pi 2022-03-02 144
59ca6c93387d32 zhenwei pi 2022-03-02 145
virtqueue_kick(vcrypto->ctrl_vq);
59ca6c93387d32 zhenwei pi 2022-03-02 146 while
(!virtqueue_get_buf(vcrypto->ctrl_vq, ) &&
59ca6c93387d32 zhenwei pi 2022-03-02 147
!virtqueue_is_broken(vcrypto-&g
Re: [PATCH AUTOSEL 5.15 13/16] vdpa: clean up get_config_size ret value handling
The mitre.org page
https://cve.mitre.org/cgi-bin/cvename.cgi?name=2022-0998
says this is a fix for CVE-2022-0998 but if you apply it by itself it
creates a serious security problem. Originally this bug only affected
32 bit systems but this patch will change it to affect everyone.
You need to apply commit 3ed21c1451a1 ("vdpa: check that offsets are
within bounds").
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3ed21c1451a14d139e1ceb18f2fa70865ce3195a
I don't know if this affects anyone, but it seemed worth mentioning.
regards,
dan carpenter
On Sat, Jan 22, 2022 at 07:12:12PM -0500, Sasha Levin wrote:
> From: Laura Abbott
>
> [ Upstream commit 870aaff92e959e29d40f9cfdb5ed06ba2fc2dae0 ]
>
> The return type of get_config_size is size_t so it makes
> sense to change the type of the variable holding its result.
>
> That said, this already got taken care of (differently, and arguably
> not as well) by commit 3ed21c1451a1 ("vdpa: check that offsets are
> within bounds").
>
> The added 'c->off > size' test in that commit will be done as an
> unsigned comparison on 32-bit (safe due to not being signed).
>
> On a 64-bit platform, it will be done as a signed comparison, but in
> that case the comparison will be done in 64-bit, and 'c->off' being an
> u32 it will be valid thanks to the extended range (ie both values will
> be positive in 64 bits).
>
> So this was a real bug, but it was already addressed and marked for stable.
>
> Signed-off-by: Laura Abbott
> Reported-by: Luo Likang
> Signed-off-by: Michael S. Tsirkin
> Signed-off-by: Sasha Levin
> ---
> drivers/vhost/vdpa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index d62f05d056b7b..913cd465f9f1e 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -195,7 +195,7 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa
> *v,
> struct vhost_vdpa_config *c)
> {
> struct vdpa_device *vdpa = v->vdpa;
> - long size = vdpa->config->get_config_size(vdpa);
> + size_t size = vdpa->config->get_config_size(vdpa);
>
> if (c->len == 0 || c->off > size)
> return -EINVAL;
> --
> 2.34.1
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vDPA/ifcvf: implement shared IRQ feature
What I really want is to re-enable GCC's uninitialized variable warning. $ rm drivers/vdpa/ifcvf/ifcvf_main.o $ make W=2 drivers/vdpa/ifcvf/ifcvf_main.o It prints a ton of output but this is the relevant bit. drivers/vdpa/ifcvf/ifcvf_main.c: In function ‘ifcvf_vdpa_set_status’: drivers/vdpa/ifcvf/ifcvf_main.c:291:6: warning: ‘config_vector’ may be used uninitialized in this function [-Wmaybe-uninitialized] int config_vector, ret; ^ regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vDPA/ifcvf: implement shared IRQ feature
On Tue, Mar 15, 2022 at 10:27:35AM +0800, Zhu, Lingshan wrote: > > > On 3/14/2022 6:37 PM, Dan Carpenter wrote: > > On Mon, Mar 14, 2022 at 10:22:03AM +0800, Zhu, Lingshan wrote: > > > Hello Dan, > > > > > > Thanks for your suggestions and this auto-testing efforts! > > > On handling the vector for device config interrupt, there are three > > > possibilities: > > > (1)it has a dedicated vector(2)it shares a vector with datapath(3)no > > > vectors. > > > > > > So in these code below, it handles the three cases, or it should be > > > -EINVAL, > > > so IMHO we don't need > > > an else there, just leave it -EINVAL. > > I'm confused about why you're talking about -EINVAL... There is no > > -EINVAL in this function. > Oh, sorry I didn't explain this well. It is a series of patches, in other > patches, we initialize hw->config_irq = -EINVAL > as a safe default value. In this function ifcvf_request_config_irq(), > hw->config_irq is generated by request_config_irq(). > > Then config_vector matters, there are only three possible values the > config_vector can be(listed above), > depending on vf->msix_vector_status. And vf->msix_vector_status depends on > how many MSI vectors we got, > > so there are only three values it can take, IMHO, it is a complete set of > the possibilities, we already > handled "else" in ifcvf_request_irq(). Alright, fine. Yes, I verified this and it's a false positive. But GCC will also warn about this code if we manage to enable the GCC warning again. If we make a chart like this: [looks correct] [ looks buggy ] [ no warnings] 1 2 [ warnings ] 3 4 Where you want to be is in box 1 where it looks correct and has no warnings. Boxes 2 and 3 are a second best option because if it doesn't have static checker warnings, people won't notice it. Or if the warnings are obvious false postives they can skip over it quickly. But this code is box 4 where it is a big waste of time for anyone running a static checker. I almost prefer actual bugs to code which is deliberately written to fit in box 4. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vDPA/ifcvf: implement shared IRQ feature
On Mon, Mar 14, 2022 at 10:22:03AM +0800, Zhu, Lingshan wrote:
> Hello Dan,
>
> Thanks for your suggestions and this auto-testing efforts!
> On handling the vector for device config interrupt, there are three
> possibilities:
> (1)it has a dedicated vector(2)it shares a vector with datapath(3)no
> vectors.
>
> So in these code below, it handles the three cases, or it should be -EINVAL,
> so IMHO we don't need
> an else there, just leave it -EINVAL.
I'm confused about why you're talking about -EINVAL... There is no
-EINVAL in this function.
This code is not necessarily buggy. Right now we have GCC uninitialized
variable warnings turned off so it also doesn't cause a build issue.
But I think we should try to work towards a future where we can
re-enable the GCC warning. GCC catches a lot of stupid uninitialized
variable bugs and it's better if we can catch them earlier instead of
relying on the kbuild-bot.
regards,
dan carpenter
>
> Thanks for your efforts!
> Zhu Lingshan
>
> On 3/11/2022 5:00 PM, Dan Carpenter wrote:
> > Hello Zhu Lingshan,
> >
> > The patch 79333575b8bd: "vDPA/ifcvf: implement shared IRQ feature"
> > from Feb 22, 2022, leads to the following Smatch static checker
> > warning:
> >
> > drivers/vdpa/ifcvf/ifcvf_main.c:306 ifcvf_request_config_irq()
> > error: uninitialized symbol 'config_vector'.> >
> > drivers/vdpa/ifcvf/ifcvf_main.c
> > 287 static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter)
> > 288 {
> > 289 struct pci_dev *pdev = adapter->pdev;
> > 290 struct ifcvf_hw *vf = >vf;
> > 291 int config_vector, ret;
> > 292
> > 293 if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED)
> > 294 return 0;
> > 295
> > 296 if (vf->msix_vector_status ==
> > MSIX_VECTOR_PER_VQ_AND_CONFIG)
> > 297 /* vector 0 ~ vf->nr_vring for vqs, num
> > vf->nr_vring vector for config interrupt */
> > 298 config_vector = vf->nr_vring;
> >
> > Set here.
> >
> > 299
> > 300 if (vf->msix_vector_status ==
> > MSIX_VECTOR_SHARED_VQ_AND_CONFIG)
> > 301 /* vector 0 for vqs and 1 for config interrupt */
> > 302 config_vector = 1;
> >
> > And here. But no else path.
> >
> > 303
> > 304 snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",
> > 305 pci_name(pdev));
> > --> 306 vf->config_irq = pci_irq_vector(pdev, config_vector);
> > 307 ret = devm_request_irq(>dev, vf->config_irq,
> > 308ifcvf_config_changed, 0,
> > 309vf->config_msix_name, vf);
> > 310 if (ret) {
> > 311 IFCVF_ERR(pdev, "Failed to request config irq\n");
> > 312 goto err;
> > 313 }
> > 314
> > 315 ret = ifcvf_set_config_vector(vf, config_vector);
> > 316 if (ret == VIRTIO_MSI_NO_VECTOR) {
> > 317 IFCVF_ERR(pdev, "No msix vector for device
> > config\n");
> > 318 goto err;
> > 319 }
> > 320
> > 321 return 0;
> > 322 err:
> > 323 ifcvf_free_irq(adapter);
> > 324
> > 325 return -EFAULT;
> > 326 }
> >
> > regards,
> > dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vDPA/ifcvf: implement shared IRQ feature
Hello Zhu Lingshan,
The patch 79333575b8bd: "vDPA/ifcvf: implement shared IRQ feature"
from Feb 22, 2022, leads to the following Smatch static checker
warning:
drivers/vdpa/ifcvf/ifcvf_main.c:306 ifcvf_request_config_irq()
error: uninitialized symbol 'config_vector'.
drivers/vdpa/ifcvf/ifcvf_main.c
287 static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter)
288 {
289 struct pci_dev *pdev = adapter->pdev;
290 struct ifcvf_hw *vf = >vf;
291 int config_vector, ret;
292
293 if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED)
294 return 0;
295
296 if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG)
297 /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring
vector for config interrupt */
298 config_vector = vf->nr_vring;
Set here.
299
300 if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG)
301 /* vector 0 for vqs and 1 for config interrupt */
302 config_vector = 1;
And here. But no else path.
303
304 snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",
305 pci_name(pdev));
--> 306 vf->config_irq = pci_irq_vector(pdev, config_vector);
307 ret = devm_request_irq(>dev, vf->config_irq,
308ifcvf_config_changed, 0,
309vf->config_msix_name, vf);
310 if (ret) {
311 IFCVF_ERR(pdev, "Failed to request config irq\n");
312 goto err;
313 }
314
315 ret = ifcvf_set_config_vector(vf, config_vector);
316 if (ret == VIRTIO_MSI_NO_VECTOR) {
317 IFCVF_ERR(pdev, "No msix vector for device config\n");
318 goto err;
319 }
320
321 return 0;
322 err:
323 ifcvf_free_irq(adapter);
324
325 return -EFAULT;
326 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] virtio_net: support rx/tx queue reset
Hello Xuan Zhuo,
The patch 26ae35c46f93: "virtio_net: support rx/tx queue reset" from
Mar 8, 2022, leads to the following Smatch static checker warning:
drivers/net/virtio_net.c:1410 virtnet_napi_tx_disable()
warn: sleeping in atomic context
drivers/net/virtio_net.c
1829static int virtnet_tx_vq_reset(struct virtnet_info *vi,
1830 struct send_queue *sq, u32
ring_num)
1831{
1832struct netdev_queue *txq;
1833int err, qindex;
1834
1835qindex = sq - vi->sq;
1836
1837txq = netdev_get_tx_queue(vi->dev, qindex);
1838__netif_tx_lock_bh(txq);
^^^
Disables preempt
1839
1840/* stop tx queue and napi */
1841netif_stop_subqueue(vi->dev, qindex);
1842virtnet_napi_tx_disable(>napi);
^^
napi_disable() is a might_sleep() function.
1843
1844__netif_tx_unlock_bh(txq);
1845
1846/* reset the queue */
1847err = virtio_reset_vq(sq->vq);
1848if (err) {
1849netif_start_subqueue(vi->dev, qindex);
1850goto err;
1851 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [syzbot] INFO: task hung in vhost_work_dev_flush
On Mon, Feb 21, 2022 at 09:23:04PM +0530, Anirudh Rayabharam wrote: > On Mon, Feb 21, 2022 at 03:12:33PM +0100, Stefano Garzarella wrote: > > #syz test: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ > > f71077a4d84b > > > > Patch sent upstream: > > https://lore.kernel.org/virtualization/20220221114916.107045-1-sgarz...@redhat.com/T/#u > > I don't see how your patch fixes this issue. It looks unrelated. It is > surprising that syzbot is happy with it. > > I have sent a patch for this issue here: > https://lore.kernel.org/lkml/20220221072852.31820-1-m...@anirudhrb.com/ I wasted so much time trying to figure out what this patch fixes. :P (It doesn't fix anything). regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost/vsock: don't check owner in vhost_vsock_stop() while releasing
Hi Stefano,
url:
https://github.com/0day-ci/linux/commits/Stefano-Garzarella/vhost-vsock-don-t-check-owner-in-vhost_vsock_stop-while-releasing/20220221-195038
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m031-20220221
(https://download.01.org/0day-ci/archive/20220222/202202220707.am3rkucp-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vsock.c:655 vhost_vsock_stop() error: uninitialized symbol 'ret'.
vim +/ret +655 drivers/vhost/vsock.c
3ace84c91bfcde Stefano Garzarella 2022-02-21 632 static int
vhost_vsock_stop(struct vhost_vsock *vsock, bool check_owner)
433fc58e6bf2c8 Asias He 2016-07-28 633 {
433fc58e6bf2c8 Asias He 2016-07-28 634 size_t i;
433fc58e6bf2c8 Asias He 2016-07-28 635 int ret;
433fc58e6bf2c8 Asias He 2016-07-28 636
433fc58e6bf2c8 Asias He 2016-07-28 637
mutex_lock(>dev.mutex);
433fc58e6bf2c8 Asias He 2016-07-28 638
3ace84c91bfcde Stefano Garzarella 2022-02-21 639 if (check_owner) {
433fc58e6bf2c8 Asias He 2016-07-28 640 ret =
vhost_dev_check_owner(>dev);
433fc58e6bf2c8 Asias He 2016-07-28 641 if (ret)
433fc58e6bf2c8 Asias He 2016-07-28 642 goto
err;
3ace84c91bfcde Stefano Garzarella 2022-02-21 643 }
"ret" not initialized on else path.
433fc58e6bf2c8 Asias He 2016-07-28 644
433fc58e6bf2c8 Asias He 2016-07-28 645 for (i = 0; i <
ARRAY_SIZE(vsock->vqs); i++) {
433fc58e6bf2c8 Asias He 2016-07-28 646 struct
vhost_virtqueue *vq = >vqs[i];
433fc58e6bf2c8 Asias He 2016-07-28 647
433fc58e6bf2c8 Asias He 2016-07-28 648
mutex_lock(>mutex);
247643f85782fc Eugenio Pérez 2020-03-31 649
vhost_vq_set_backend(vq, NULL);
433fc58e6bf2c8 Asias He 2016-07-28 650
mutex_unlock(>mutex);
433fc58e6bf2c8 Asias He 2016-07-28 651 }
433fc58e6bf2c8 Asias He 2016-07-28 652
433fc58e6bf2c8 Asias He 2016-07-28 653 err:
433fc58e6bf2c8 Asias He 2016-07-28 654
mutex_unlock(>dev.mutex);
433fc58e6bf2c8 Asias He 2016-07-28 @655 return ret;
433fc58e6bf2c8 Asias He 2016-07-28 656 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] new kvmalloc() WARN() triggered by DRM ioctls tracking
Hi DRM Devs,
In commit 7661809d493b ("mm: don't allow oversized kvmalloc() calls")
from July, Linus added a WARN_ONCE() for "crazy" allocations over 2GB.
I have a static checker warning for this and most of the warnings are
from DRM ioctls.
drivers/gpu/drm/lima/lima_drv.c:124 lima_ioctl_gem_submit() warn: uncapped user
size for kvmalloc() will WARN
drivers/gpu/drm/radeon/radeon_cs.c:291 radeon_cs_parser_init() warn: uncapped
user size for kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:311 v3d_lookup_bos() warn: uncapped user size for
kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:319 v3d_lookup_bos() warn: uncapped user size for
kvmalloc() will WARN
drivers/gpu/drm/v3d/v3d_gem.c:601 v3d_get_multisync_post_deps() warn: uncapped
user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:476 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:477 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:478 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c:479 etnaviv_ioctl_gem_submit()
warn: uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/virtio/virtgpu_ioctl.c:186 virtio_gpu_execbuffer_ioctl() warn:
uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/panfrost/panfrost_drv.c:198 panfrost_copy_in_sync() warn:
uncapped user size for kvmalloc() will WARN
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:120 amdgpu_cs_parser_init() warn:
uncapped user size for kvmalloc() will WARN
These ioctls can all trigger the stack dump. The line numbers are from
linux next (next-20211214).
I feel like ideally if this could be fixed in a central way, but if not
then hopefully I've added the relevant lists to the CC.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vduse: check that offset is within bounds in get_config()
This condition checks "len" but it does not check "offset" and that
could result in an out of bounds read if "offset > dev->config_size".
The problem is that since both variables are unsigned the
"dev->config_size - offset" subtraction would result in a very high
unsigned value.
I think these checks might not be necessary because "len" and "offset"
are supposed to already have been validated using the
vhost_vdpa_config_validate() function. But I do not know the code
perfectly, and I like to be safe.
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index c9204c62f339..6693a2c9e4a6 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -655,7 +655,8 @@ static void vduse_vdpa_get_config(struct vdpa_device *vdpa,
unsigned int offset,
{
struct vduse_dev *dev = vdpa_to_vduse(vdpa);
- if (len > dev->config_size - offset)
+ if (offset > dev->config_size ||
+ len > dev->config_size - offset)
return;
memcpy(buf, dev->config + offset, len);
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/2 v4] vdpa: check that offsets are within bounds
In this function "c->off" is a u32 and "size" is a long. On 64bit systems
if "c->off" is greater than "size" then "size - c->off" is a negative and
we always return -E2BIG. But on 32bit systems the subtraction is type
promoted to a high positive u32 value and basically any "c->len" is
accepted.
Fixes: 4c8cf31885f6 ("vhost: introduce vDPA-based backend")
Reported-by: Xie Yongji
Signed-off-by: Dan Carpenter
---
v4: split into a separate patch
drivers/vhost/vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 29cced1cd277..e3c4f059b21a 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -197,7 +197,7 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa *v,
struct vdpa_device *vdpa = v->vdpa;
long size = vdpa->config->get_config_size(vdpa);
- if (c->len == 0)
+ if (c->len == 0 || c->off > size)
return -EINVAL;
if (c->len > size - c->off)
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/2 v4] vduse: fix memory corruption in vduse_dev_ioctl()
The "config.offset" comes from the user. There needs to a check to
prevent it being out of bounds. The "config.offset" and
"dev->config_size" variables are both type u32. So if the offset if
out of bounds then the "dev->config_size - config.offset" subtraction
results in a very high u32 value. The out of bounds offset can result
in memory corruption.
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Dan Carpenter
---
v2: fix reversed if statement
v3: fix vhost_vdpa_config_validate() as pointed out by Yongji Xie.
v4: split the vhost_vdpa_config_validate() change into a separate path
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index c9204c62f339..1a206f95d73a 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int
cmd,
break;
ret = -EINVAL;
- if (config.length == 0 ||
+ if (config.offset > dev->config_size ||
+ config.length == 0 ||
config.length > dev->config_size - config.offset)
break;
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3] vduse: vduse: check that offsets are within bounds
In vduse_dev_ioctl(), the "config.offset" comes from the user. There
needs to a check to prevent it being out of bounds. The "config.offset"
and "dev->config_size" variables are both type u32. So if the offset is
out of bounds then the "dev->config_size - config.offset" subtraction
results in a very high u32 value.
The vhost_vdpa_config_validate() function has a similar issue, but there
the "size" is long type so the subtraction works on 64bit system and
this change only affects 32bit systems.
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Fixes: 4c8cf31885f6 ("vhost: introduce vDPA-based backend")
Signed-off-by: Dan Carpenter
---
v2: the first version had a reversed if statement
v3: fix vhost_vdpa_config_validate() as pointed out by Yongji Xie.
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
drivers/vhost/vdpa.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index c9204c62f339..1a206f95d73a 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int
cmd,
break;
ret = -EINVAL;
- if (config.length == 0 ||
+ if (config.offset > dev->config_size ||
+ config.length == 0 ||
config.length > dev->config_size - config.offset)
break;
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 29cced1cd277..e3c4f059b21a 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -197,7 +197,7 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa *v,
struct vdpa_device *vdpa = v->vdpa;
long size = vdpa->config->get_config_size(vdpa);
- if (c->len == 0)
+ if (c->len == 0 || c->off > size)
return -EINVAL;
if (c->len > size - c->off)
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vduse: vduse: fix memory corruption in vduse_dev_ioctl()
On Tue, Dec 07, 2021 at 02:21:46PM +0300, Dan Carpenter wrote:
> The "config.offset" comes from the user. There needs to a check to
> prevent it being out of bounds. The "config.offset" and
> "dev->config_size" variables are both type u32. So if the offset if
> out of bounds then the "dev->config_size - config.offset" subtraction
> results in a very high u32 value.
>
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Signed-off-by: Dan Carpenter
> ---
> v2: version 1 had a reversed if statement
Xie Yongji pointed out that vhost_vdpa_config_validate() had a similar
issue so I'll send a v3.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vduse: fix memory corruption in vduse_dev_ioctl()
On Tue, Dec 07, 2021 at 07:19:35PM +0800, Yongji Xie wrote: > On Tue, Dec 7, 2021 at 6:46 PM Dan Carpenter wrote: > > > > The "config.offset" comes from the user. There needs to a check to > > prevent it being out of bounds. The "config.offset" and > > "dev->config_size" variables are both type u32. So if the offset if > > out of bounds then the "dev->config_size - config.offset" subtraction > > results in a very high u32 value. > > > > Thanks for catching this! I think we also need a fix for > vhost_vdpa_config_validate(). Okay. I just sent v2. I'll send a v3. vhost_vdpa_config_validate() uses long type for "size" so the subtract works okay on a 64bit system. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2] vduse: vduse: fix memory corruption in vduse_dev_ioctl()
The "config.offset" comes from the user. There needs to a check to
prevent it being out of bounds. The "config.offset" and
"dev->config_size" variables are both type u32. So if the offset if
out of bounds then the "dev->config_size - config.offset" subtraction
results in a very high u32 value.
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Dan Carpenter
---
v2: version 1 had a reversed if statement
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index c9204c62f339..1a206f95d73a 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int
cmd,
break;
ret = -EINVAL;
- if (config.length == 0 ||
+ if (config.offset > dev->config_size ||
+ config.length == 0 ||
config.length > dev->config_size - config.offset)
break;
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vduse: fix memory corruption in vduse_dev_ioctl()
On Tue, Dec 07, 2021 at 01:46:14PM +0300, Dan Carpenter wrote:
> The "config.offset" comes from the user. There needs to a check to
> prevent it being out of bounds. The "config.offset" and
> "dev->config_size" variables are both type u32. So if the offset if
> out of bounds then the "dev->config_size - config.offset" subtraction
> results in a very high u32 value.
>
> Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
> Signed-off-by: Dan Carpenter
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> b/drivers/vdpa/vdpa_user/vduse_dev.c
> index c9204c62f339..2f789dca0c0b 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned
> int cmd,
> break;
>
> ret = -EINVAL;
> - if (config.length == 0 ||
> + if (config.offset < dev->config_size ||
This is reversed. Sorry. Will send a fix. I was over excited about
this patch.
Also I need to write a Smatch test for this so both the original and my
patched version generate a warning.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vduse: fix memory corruption in vduse_dev_ioctl()
The "config.offset" comes from the user. There needs to a check to
prevent it being out of bounds. The "config.offset" and
"dev->config_size" variables are both type u32. So if the offset if
out of bounds then the "dev->config_size - config.offset" subtraction
results in a very high u32 value.
Fixes: c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index c9204c62f339..2f789dca0c0b 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int
cmd,
break;
ret = -EINVAL;
- if (config.length == 0 ||
+ if (config.offset < dev->config_size ||
+ config.length == 0 ||
config.length > dev->config_size - config.offset)
break;
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
I appologize. This thread has been really frustrating. I got mixed up
because I recently sent patches for ingenic and vc4. Also we are
working against different trees so maybe that is part of the problem?
I'm looking at today's linux-next. Your patch has been applied.
Yes. You updated all the drivers. But somehow the vc4 chunk from your
patch was dropped. It was *NOT* dropped by Stephen Rothwell. It got
dropped earlier. I am including the `git format-patch -1 ` output
from the commit.
regards,
dan carpenter
>From 4ff22f487f8c26b99cbe1678344595734c001a39 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann
Date: Tue, 30 Nov 2021 10:52:55 +0100
Subject: [PATCH] drm: Return error codes from struct
drm_driver.gem_create_object
GEM helper libraries use struct drm_driver.gem_create_object to let
drivers override GEM object allocation. On failure, the call returns
NULL.
Change the semantics to make the calls return a pointer-encoded error.
This aligns the callback with its callers. Fixes the ingenic driver,
which already returns an error pointer.
Also update the callers to handle the involved types more strictly.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Steven Price
Acked-by: Maxime Ripard
Link:
https://patchwork.freedesktop.org/patch/msgid/20211130095255.26710-1-tzimmerm...@suse.de
---
drivers/gpu/drm/drm_gem_cma_helper.c| 17 ++---
drivers/gpu/drm/drm_gem_shmem_helper.c | 17 ++---
drivers/gpu/drm/drm_gem_vram_helper.c | 4 ++--
drivers/gpu/drm/lima/lima_gem.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
drivers/gpu/drm/v3d/v3d_bo.c| 4 ++--
drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
drivers/gpu/drm/virtio/virtgpu_object.c | 2 +-
include/drm/drm_drv.h | 5 +++--
9 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
b/drivers/gpu/drm/drm_gem_cma_helper.c
index 5f42e44b2ab3..6f18f143dd30 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size,
bool private)
struct drm_gem_object *gem_obj;
int ret = 0;
- if (drm->driver->gem_create_object)
+ if (drm->driver->gem_create_object) {
gem_obj = drm->driver->gem_create_object(drm, size);
- else
- gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
- if (!gem_obj)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(gem_obj))
+ return ERR_CAST(gem_obj);
+ cma_obj = to_drm_gem_cma_obj(gem_obj);
+ } else {
+ cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL);
+ if (!cma_obj)
+ return ERR_PTR(-ENOMEM);
+ gem_obj = _obj->base;
+ }
if (!gem_obj->funcs)
gem_obj->funcs = _gem_cma_default_funcs;
- cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base);
-
if (private) {
drm_gem_private_object_init(drm, gem_obj, size);
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 0eeda1012364..7915047cb041 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size,
bool private)
size = PAGE_ALIGN(size);
- if (dev->driver->gem_create_object)
+ if (dev->driver->gem_create_object) {
obj = dev->driver->gem_create_object(dev, size);
- else
- obj = kzalloc(sizeof(*shmem), GFP_KERNEL);
- if (!obj)
- return ERR_PTR(-ENOMEM);
-
- shmem = to_drm_gem_shmem_obj(obj);
+ if (IS_ERR(obj))
+ return ERR_CAST(obj);
+ shmem = to_drm_gem_shmem_obj(obj);
+ } else {
+ shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+ if (!shmem)
+ return ERR_PTR(-ENOMEM);
+ obj = >base;
+ }
if (!obj->funcs)
obj->funcs = _gem_shmem_funcs;
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
b/drivers/gpu/drm/drm_gem_vram_helper.c
index bfa386b98134..3f00192215d1 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct
drm_device *dev,
if (dev->driver->gem_create_object) {
gem = dev->driver->gem_create_object(dev, size);
- if (!gem)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(gem))
+ return ERR_CAST(gem);
gbo = drm_gem_vram_of_gem(gem);
} else {
gbo = kzalloc(sizeof(*gbo), GFP_KERNEL)
Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote: > Hi > > Am 06.12.21 um 11:42 schrieb Dan Carpenter: > > On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote: > > > GEM helper libraries use struct drm_driver.gem_create_object to let > > > drivers override GEM object allocation. On failure, the call returns > > > NULL. > > > > > > Change the semantics to make the calls return a pointer-encoded error. > > > This aligns the callback with its callers. Fixes the ingenic driver, > > > which already returns an error pointer. > > > > > > Also update the callers to handle the involved types more strictly. > > > > > > Signed-off-by: Thomas Zimmermann > > > --- > > > There is an alternative patch at [1] that updates the value returned > > > by ingenics' gem_create_object to NULL. Fixing the interface to return > > > an errno code is more consistent with the rest of the GEM functions. > > > > > > [1] https://lore.kernel.org/dri-devel/2028111522.GD1147@kili/ > > > > My fix was already applied and backported to -stable etc... Your > > patch is not developed against a current tree so you broke it. > > Do you have a specific link? I just checked the stable tree at [1] and there > no trace of your patch. It's in 5.15.6 and probably all the other supported -stable trees. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387 > > Patches for DRM should go through through DRM trees; drm-misc-fixes in this > case. Exceptions should at least be announce on dri-devel. Neither is the > case here. Yeah. That's a good question. I don't know, because I just work against linux-next... regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote: > GEM helper libraries use struct drm_driver.gem_create_object to let > drivers override GEM object allocation. On failure, the call returns > NULL. > > Change the semantics to make the calls return a pointer-encoded error. > This aligns the callback with its callers. Fixes the ingenic driver, > which already returns an error pointer. > > Also update the callers to handle the involved types more strictly. > > Signed-off-by: Thomas Zimmermann > --- > There is an alternative patch at [1] that updates the value returned > by ingenics' gem_create_object to NULL. Fixing the interface to return > an errno code is more consistent with the rest of the GEM functions. > > [1] https://lore.kernel.org/dri-devel/2028111522.GD1147@kili/ My fix was already applied and backported to -stable etc... Your patch is not developed against a current tree so you broke it. That's the tricky thing with changing the API because say people wrote their code last week where returning NULL was correct. When they submit their driver upstream, everything will merge and build but it will break at runtime. For now, it's only vc4_create_object() which is broken. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa: Consider device id larger than 31
On Sun, Nov 28, 2021 at 09:14:35AM +0200, Eli Cohen wrote:
> On Fri, Nov 26, 2021 at 10:48:12AM +0800, Jason Wang wrote:
> > On Fri, Nov 26, 2021 at 2:09 AM Parav Pandit wrote:
> > >
> > > virtio device id value can be more than 31. Hence, use BIT_ULL in
> > > assignment.
> > >
> > > Fixes: 33b347503f01 ("vdpa: Define vdpa mgmt device, ops and a netlink
> > > interface")
> > > Reported-by: kernel test robot
> > > Reported-by: Dan Carpenter
> > > Signed-off-by: Parav Pandit
> >
> > Acked-by: Jason Wang
> >
> > > ---
> > > drivers/vdpa/vdpa.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > index 7332a74a4b00..e91c71aeeddf 100644
> > > --- a/drivers/vdpa/vdpa.c
> > > +++ b/drivers/vdpa/vdpa.c
> > > @@ -404,7 +404,7 @@ static int vdpa_mgmtdev_fill(const struct
> > > vdpa_mgmt_dev *mdev, struct sk_buff *m
> > > goto msg_err;
> > >
> > > while (mdev->id_table[i].device) {
> > > - supported_classes |= BIT(mdev->id_table[i].device);
> > > + supported_classes |= BIT_ULL(mdev->id_table[i].device);
> > > i++;
> > > }
> > >
>
> type of mdev->id_table[i].device is __u32 so in theory you're limited
> to device ID's up to 63.
A u32 can fit numbers up to 4 million? These .device numbers are
normally hardcoded defines listed in usr/include/linux/virtio_ids.h
But sometimes they're not like in vp_modern_probe() which does:
mdev->id.device = pci_dev->device - 0x1040;
I don't know if an assert is really worth it, considering how almost all
of them are hardcoded. Also if we do want an assert maybe there is a
better place to put it?
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2] drm/virtio: Fix an NULL vs IS_ERR() bug in virtio_gpu_object_shmem_init()
The drm_gem_shmem_get_sg_table() function never returns NULL. It returns
error pointers on error.
Fixes: c66df701e783 ("drm/virtio: switch from ttm to gem shmem helpers")
Signed-off-by: Dan Carpenter
---
v2: I originally sent this patch on 19 Jun 2020 but it was somehow
not applied. As I review it now, I see that the bug is actually
older than I originally thought and so I have updated the Fixes
tag.
drivers/gpu/drm/virtio/virtgpu_object.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index f648b0e24447..8bb80289672c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -168,9 +168,9 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
* since virtio_gpu doesn't support dma-buf import from other devices.
*/
shmem->pages = drm_gem_shmem_get_sg_table(>base.base);
- if (!shmem->pages) {
+ if (IS_ERR(shmem->pages)) {
drm_gem_shmem_unpin(>base.base);
- return -EINVAL;
+ return PTR_ERR(shmem->pages);
}
if (use_dma_api) {
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vdpa_sim_blk: implement ramdisk behaviour
On Wed, Sep 29, 2021 at 02:07:12PM +0200, Stefano Garzarella wrote:
> On Wed, Sep 29, 2021 at 02:46:52PM +0300, Dan Carpenter wrote:
> > On Wed, Sep 29, 2021 at 02:37:42PM +0300, Dan Carpenter wrote:
> > > 89 /* The last byte is the status and we checked if the last
> > > iov has
> > > 90 * enough room for it.
> > > 91 */
> > > 92 to_push = vringh_kiov_length(>in_iov) - 1;
> > >
> > > Are you positive that vringh_kiov_length() cannot be zero? I looked at
> > > the range_check() and there is no check for "if (*len == 0)".
> > >
> > > 93
> > > 94 to_pull = vringh_kiov_length(>out_iov);
> > > 95
> > > 96 bytes = vringh_iov_pull_iotlb(>vring, >out_iov,
> > > ,
> > > 97 sizeof(hdr));
> > > 98 if (bytes != sizeof(hdr)) {
> > > 99 dev_err(>vdpa.dev, "request out header
> > > too short\n");
> > > 100 return false;
> > > 101 }
> > > 102
> > > 103 to_pull -= bytes;
> > >
> > > The same "bytes" is used for both to_pull and to_push. In this
> > > assignment it would only be used for the default case which prints an
> > > error message.
> > >
> >
> > Sorry, no. This part is wrong. "bytes" is not used for "to_push"
> > either here or below. But I still am not sure "*len == 0" or how we
>
> At line 84 we check that the last `in_iov` has at least one byte, so
> vringh_kiov_length(>in_iov) cannot be zero.
> It will return the sum of all lengths, so at least 1.
>
> Maybe better to add a comment.
>
> > know that "to_push >= VIRTIO_BLK_ID_BYTES".
>
> vringh_iov_push_iotlb() will push at least the bytes available in `in_iov`,
> if these are less, it will copy less bytes of VIRTIO_BLK_ID_BYTES.
>
> Maybe here it would be better to add a check because the driver isn't
> following the specification.
>
> And I'd avoid the subtraction highlighted by Smatch static checker.
>
> Thanks for reporting. I'll send patches to fix these issues.
Nothing to fix, really. I've looked at what you've explained and it's
all true so the code is fine as-is. Thanks so much.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vdpa_sim_blk: implement ramdisk behaviour
On Wed, Sep 29, 2021 at 02:37:42PM +0300, Dan Carpenter wrote:
> 89 /* The last byte is the status and we checked if the last iov
> has
> 90 * enough room for it.
> 91 */
> 92 to_push = vringh_kiov_length(>in_iov) - 1;
>
> Are you positive that vringh_kiov_length() cannot be zero? I looked at
> the range_check() and there is no check for "if (*len == 0)".
>
> 93
> 94 to_pull = vringh_kiov_length(>out_iov);
> 95
> 96 bytes = vringh_iov_pull_iotlb(>vring, >out_iov, ,
> 97 sizeof(hdr));
> 98 if (bytes != sizeof(hdr)) {
> 99 dev_err(>vdpa.dev, "request out header too
> short\n");
> 100 return false;
> 101 }
> 102
> 103 to_pull -= bytes;
>
> The same "bytes" is used for both to_pull and to_push. In this
> assignment it would only be used for the default case which prints an
> error message.
>
Sorry, no. This part is wrong. "bytes" is not used for "to_push"
either here or below. But I still am not sure "*len == 0" or how we
know that "to_push >= VIRTIO_BLK_ID_BYTES".
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vdpa_sim_blk: implement ramdisk behaviour
status = VIRTIO_BLK_S_IOERR;
151 break;
152 }
153 break;
154
155 case VIRTIO_BLK_T_GET_ID:
156 bytes = vringh_iov_push_iotlb(>vring, >in_iov,
157 vdpasim_blk_id,
158 VIRTIO_BLK_ID_BYTES);
Do we know that to_push is larger than VIRTIO_BLK_ID_BYTES?
159 if (bytes < 0) {
160 dev_err(>vdpa.dev,
161 "vringh_iov_push_iotlb() error: %zd\n",
bytes);
162 status = VIRTIO_BLK_S_IOERR;
163 break;
164 }
165
166 pushed += bytes;
167 break;
168
169 default:
170 dev_warn(>vdpa.dev,
171 "Unsupported request type %d\n", type);
172 status = VIRTIO_BLK_S_IOERR;
173 break;
174 }
175
176 /* If some operations fail, we need to skip the remaining bytes
177 * to put the status in the last byte
178 */
--> 179 if (to_push - pushed > 0)
What I'm doing here is I'm looking for places where there are signedness
bugs such that "pushed > to_push" and the subtraction results in a high
positive value instead of a negative value.
I think there are potential issues here. It's definitely not clear
without reading a lot of context whether this code is safe.
180 vringh_kiov_advance(>in_iov, to_push - pushed);
181
182 /* Last byte is the status */
183 bytes = vringh_iov_push_iotlb(>vring, >in_iov, ,
1);
184 if (bytes != 1)
185 return false;
186
187 pushed += bytes;
188
189 /* Make sure data is wrote before advancing index */
190 smp_wmb();
191
192 vringh_complete_iotlb(>vring, vq->head, pushed);
193
194 return true;
195 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpa: potential uninitialized return in vhost_vdpa_va_map()
The concern here is that "ret" can be uninitialized if we hit the
"goto next" condition on every iteration through the loop.
Fixes: 41ba1b5f9d4b ("vdpa: Support transferring virtual addressing during DMA
mapping")
Signed-off-by: Dan Carpenter
---
drivers/vhost/vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index d0c0fedf2c09..170166806714 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -640,7 +640,7 @@ static int vhost_vdpa_va_map(struct vhost_vdpa *v,
u64 offset, map_size, map_iova = iova;
struct vdpa_map_file *map_file;
struct vm_area_struct *vma;
- int ret;
+ int ret = 0;
mmap_read_lock(dev->mm);
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vduse: missing error code in vduse_init()
This should return -ENOMEM if alloc_workqueue() fails. Currently it
returns success.
Fixes: b66219796563 ("vduse: Introduce VDUSE - vDPA Device in Userspace")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_user/vduse_dev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 5c25ff6483ad..fcd7de8dd1f2 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1593,8 +1593,10 @@ static int vduse_init(void)
vduse_irq_wq = alloc_workqueue("vduse-irq",
WQ_HIGHPRI | WQ_SYSFS | WQ_UNBOUND, 0);
- if (!vduse_irq_wq)
+ if (!vduse_irq_wq) {
+ ret = -ENOMEM;
goto err_wq;
+ }
ret = vduse_domain_init();
if (ret)
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()
Hello David Hildenbrand,
The patch 6cc26d77613a: "virtio-mem: use page_offline_(start|end)
when setting PageOffline()" from Jun 30, 2021, leads to the following
Smatch static checker warning:
drivers/virtio/virtio_mem.c:1072 virtio_mem_set_fake_offline()
warn: sleeping in atomic context
drivers/virtio/virtio_mem.c
1069 static void virtio_mem_set_fake_offline(unsigned long pfn,
1070unsigned long nr_pages, bool
onlined)
1071 {
--> 1072page_offline_begin();
virtio_mem_online_page_cb() is holding rcu_read_lock() so calling
page_offline_begin() here is sleeping in atomic bug.
1073for (; nr_pages--; pfn++) {
1074struct page *page = pfn_to_page(pfn);
1075
1076__SetPageOffline(page);
1077if (!onlined) {
1078SetPageDirty(page);
1079/* FIXME: remove after cleanups */
1080ClearPageReserved(page);
1081}
1082}
1083page_offline_end();
1084 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [bug report] vhost_net: basic polling support
On Wed, Aug 11, 2021 at 08:51:22PM +0800, Jason Wang wrote:
> Hi Dan:
>
> On Wed, Aug 11, 2021 at 8:14 PM Dan Carpenter
> wrote:
> >
> > Hello Jason Wang,
> >
> > The patch 030881372460: "vhost_net: basic polling support" from Mar
> > 4, 2016, leads to the following
> > Smatch static checker warning:
> >
> > drivers/vhost/vhost.c:2565 vhost_new_msg()
> > warn: sleeping in atomic context
> >
> > vers/vhost/net.c
> >509 static void vhost_net_busy_poll(struct vhost_net *net,
> >510 struct vhost_virtqueue *rvq,
> >511 struct vhost_virtqueue *tvq,
> >512 bool *busyloop_intr,
> >513 bool poll_rx)
> >514 {
> >515 unsigned long busyloop_timeout;
> >516 unsigned long endtime;
> >517 struct socket *sock;
> >518 struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> >519
> >520 /* Try to hold the vq mutex of the paired virtqueue. We
> > can't
> >521 * use mutex_lock() here since we could not guarantee a
> >522 * consistenet lock ordering.
> >523 */
> >524 if (!mutex_trylock(>mutex))
> >525 return;
> >526
> >527 vhost_disable_notify(>dev, vq);
> >528 sock = vhost_vq_get_backend(rvq);
> >529
> >530 busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
> >531 tvq->busyloop_timeout;
> >532
> >533 preempt_disable();
> > ^
> > This bumps the preemp_count.
> >
> > I guess this is to disable page faults?
>
> No, it's intended since we will use busy_clock() which uses the per
> cpu variable.
>
> >
> >534 endtime = busy_clock() + busyloop_timeout;
> >535
> >536 while (vhost_can_busy_poll(endtime)) {
> >537 if (vhost_has_work(>dev)) {
> >538 *busyloop_intr = true;
> >539 break;
> >540 }
> >541
> >542 if ((sock_has_rx_data(sock) &&
> >543 !vhost_vq_avail_empty(>dev, rvq)) ||
> >
> > The call tree from here to the GFP_KERNEL is very long...
> >
> > vhost_vq_avail_empty()
> > -> vhost_get_avail_idx()
> >-> __vhost_get_user()
> > -> __vhost_get_user_slow()
> > -> translate_desc()
> > -> vhost_iotlb_miss vhost_new_msg() <-- GFP_KERNEL
>
> This won't happen in reality since:
>
> We will make sure the IOTLB contains the translation for avail idx.
> See vq_meta_prefetch() that will be called at the begining of
> handle_tx() and handle_rx().
>
> So it looks like a false positive to me.
Thanks for looking at this. These warnings are very complicated for
me to review because of the long call trees... Smatch is pretty good
at tracking state within a function but at the function boundaries a
lot of state is lost.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vhost_net: basic polling support
Hello Jason Wang,
The patch 030881372460: "vhost_net: basic polling support" from Mar
4, 2016, leads to the following
Smatch static checker warning:
drivers/vhost/vhost.c:2565 vhost_new_msg()
warn: sleeping in atomic context
vers/vhost/net.c
509 static void vhost_net_busy_poll(struct vhost_net *net,
510 struct vhost_virtqueue *rvq,
511 struct vhost_virtqueue *tvq,
512 bool *busyloop_intr,
513 bool poll_rx)
514 {
515 unsigned long busyloop_timeout;
516 unsigned long endtime;
517 struct socket *sock;
518 struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
519
520 /* Try to hold the vq mutex of the paired virtqueue. We can't
521 * use mutex_lock() here since we could not guarantee a
522 * consistenet lock ordering.
523 */
524 if (!mutex_trylock(>mutex))
525 return;
526
527 vhost_disable_notify(>dev, vq);
528 sock = vhost_vq_get_backend(rvq);
529
530 busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
531 tvq->busyloop_timeout;
532
533 preempt_disable();
^
This bumps the preemp_count.
I guess this is to disable page faults?
534 endtime = busy_clock() + busyloop_timeout;
535
536 while (vhost_can_busy_poll(endtime)) {
537 if (vhost_has_work(>dev)) {
538 *busyloop_intr = true;
539 break;
540 }
541
542 if ((sock_has_rx_data(sock) &&
543 !vhost_vq_avail_empty(>dev, rvq)) ||
The call tree from here to the GFP_KERNEL is very long...
vhost_vq_avail_empty()
-> vhost_get_avail_idx()
-> __vhost_get_user()
-> __vhost_get_user_slow()
-> translate_desc()
-> vhost_iotlb_miss vhost_new_msg() <-- GFP_KERNEL
544 !vhost_vq_avail_empty(>dev, tvq))
545 break;
546
547 cpu_relax();
548 }
549
550 preempt_enable();
551
552 if (poll_rx || sock_has_rx_data(sock))
553 vhost_net_busy_poll_try_queue(net, vq);
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
On Wed, Jul 14, 2021 at 05:41:54PM +0800, Jason Wang wrote:
>
> 在 2021/7/14 下午4:05, Dan Carpenter 写道:
> > On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote:
> > > 在 2021/7/13 下午7:31, Dan Carpenter 写道:
> > > > On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote:
> > > > > @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa
> > > > > *v, u64 iova, u64 size)
> > > > > }
> > > > >}
> > > > > -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
> > > > > -struct vhost_iotlb_msg *msg)
> > > > > +static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
> > > > > + u64 iova, u64 size, u64 uaddr, u32 perm)
> > > > >{
> > > > > struct vhost_dev *dev = >vdev;
> > > > > - struct vhost_iotlb *iotlb = dev->iotlb;
> > > > > struct page **page_list;
> > > > > unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
> > > > > unsigned int gup_flags = FOLL_LONGTERM;
> > > > > unsigned long npages, cur_base, map_pfn, last_pfn = 0;
> > > > > unsigned long lock_limit, sz2pin, nchunks, i;
> > > > > - u64 iova = msg->iova;
> > > > > + u64 start = iova;
> > > > > long pinned;
> > > > > int ret = 0;
> > > > > - if (msg->iova < v->range.first ||
> > > > > - msg->iova + msg->size - 1 > v->range.last)
> > > > > - return -EINVAL;
> > > > This is not related to your patch, but can the "msg->iova + msg->size"
> > > > addition can have an integer overflow. From looking at the callers it
> > > > seems like it can. msg comes from:
> > > > vhost_chr_write_iter()
> > > > --> dev->msg_handler(dev, );
> > > > --> vhost_vdpa_process_iotlb_msg()
> > > >--> vhost_vdpa_process_iotlb_update()
> > >
> > > Yes.
> > >
> > >
> > > > If I'm thinking of the right thing then these are allowed to overflow to
> > > > 0 because of the " - 1" but not further than that. I believe the check
> > > > needs to be something like:
> > > >
> > > > if (msg->iova < v->range.first ||
> > > > msg->iova - 1 > U64_MAX - msg->size ||
> > >
> > > I guess we don't need - 1 here?
> > The - 1 is important. The highest address is 0x. So it goes
> > start + size = 0 and then start + size - 1 == 0x.
>
>
> Right, so actually
>
> msg->iova = 0xfffe, msg->size=2 is valid.
I believe so, yes. It's inclusive of 0xfffe and 0x.
(Not an expert).
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
On Wed, Jul 14, 2021 at 10:14:32AM +0800, Jason Wang wrote:
>
> 在 2021/7/13 下午7:31, Dan Carpenter 写道:
> > On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote:
> > > @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
> > > u64 iova, u64 size)
> > > }
> > > }
> > > -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
> > > -struct vhost_iotlb_msg *msg)
> > > +static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
> > > + u64 iova, u64 size, u64 uaddr, u32 perm)
> > > {
> > > struct vhost_dev *dev = >vdev;
> > > - struct vhost_iotlb *iotlb = dev->iotlb;
> > > struct page **page_list;
> > > unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
> > > unsigned int gup_flags = FOLL_LONGTERM;
> > > unsigned long npages, cur_base, map_pfn, last_pfn = 0;
> > > unsigned long lock_limit, sz2pin, nchunks, i;
> > > - u64 iova = msg->iova;
> > > + u64 start = iova;
> > > long pinned;
> > > int ret = 0;
> > > - if (msg->iova < v->range.first ||
> > > - msg->iova + msg->size - 1 > v->range.last)
> > > - return -EINVAL;
> > This is not related to your patch, but can the "msg->iova + msg->size"
> > addition can have an integer overflow. From looking at the callers it
> > seems like it can. msg comes from:
> >vhost_chr_write_iter()
> >--> dev->msg_handler(dev, );
> >--> vhost_vdpa_process_iotlb_msg()
> > --> vhost_vdpa_process_iotlb_update()
>
>
> Yes.
>
>
> >
> > If I'm thinking of the right thing then these are allowed to overflow to
> > 0 because of the " - 1" but not further than that. I believe the check
> > needs to be something like:
> >
> > if (msg->iova < v->range.first ||
> > msg->iova - 1 > U64_MAX - msg->size ||
>
>
> I guess we don't need - 1 here?
The - 1 is important. The highest address is 0x. So it goes
start + size = 0 and then start + size - 1 == 0x.
I guess we could move the - 1 to the other side?
msg->iova > U64_MAX - msg->size + 1 ||
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
On Tue, Jul 13, 2021 at 04:46:55PM +0800, Xie Yongji wrote:
> +static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
> +{
> + struct vduse_vdpa *vdev;
> + int ret;
> +
> + if (dev->vdev)
> + return -EEXIST;
> +
> + vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
> + _vdpa_config_ops, name, true);
> + if (!vdev)
> + return -ENOMEM;
This should be an IS_ERR() check instead of a NULL check.
The vdpa_alloc_device() macro is doing something very complicated but
I'm not sure what. It calls container_of() and that looks buggy until
you spot the BUILD_BUG_ON_ZERO() compile time assert which ensures that
the container_of() is a no-op.
Only one of the callers checks for error pointers correctly so maybe
it's too complicated or maybe there should be better documentation.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 13/17] vdpa: factor out vhost_vdpa_pa_map() and vhost_vdpa_pa_unmap()
On Tue, Jul 13, 2021 at 04:46:52PM +0800, Xie Yongji wrote:
> @@ -613,37 +618,28 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64
> iova, u64 size)
> }
> }
>
> -static int vhost_vdpa_process_iotlb_update(struct vhost_vdpa *v,
> -struct vhost_iotlb_msg *msg)
> +static int vhost_vdpa_pa_map(struct vhost_vdpa *v,
> + u64 iova, u64 size, u64 uaddr, u32 perm)
> {
> struct vhost_dev *dev = >vdev;
> - struct vhost_iotlb *iotlb = dev->iotlb;
> struct page **page_list;
> unsigned long list_size = PAGE_SIZE / sizeof(struct page *);
> unsigned int gup_flags = FOLL_LONGTERM;
> unsigned long npages, cur_base, map_pfn, last_pfn = 0;
> unsigned long lock_limit, sz2pin, nchunks, i;
> - u64 iova = msg->iova;
> + u64 start = iova;
> long pinned;
> int ret = 0;
>
> - if (msg->iova < v->range.first ||
> - msg->iova + msg->size - 1 > v->range.last)
> - return -EINVAL;
This is not related to your patch, but can the "msg->iova + msg->size"
addition can have an integer overflow. From looking at the callers it
seems like it can. msg comes from:
vhost_chr_write_iter()
--> dev->msg_handler(dev, );
--> vhost_vdpa_process_iotlb_msg()
--> vhost_vdpa_process_iotlb_update()
If I'm thinking of the right thing then these are allowed to overflow to
0 because of the " - 1" but not further than that. I believe the check
needs to be something like:
if (msg->iova < v->range.first ||
msg->iova - 1 > U64_MAX - msg->size ||
msg->iova + msg->size - 1 > v->range.last)
But writing integer overflow check correctly is notoriously difficult.
Do you think you could send a fix for that which is separate from the
patcheset? We'd want to backport it to stable.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 07/17] virtio: Don't set FAILED status bit on device index allocation failure
On Tue, Jul 13, 2021 at 04:46:46PM +0800, Xie Yongji wrote: > We don't need to set FAILED status bit on device index allocation > failure since the device initialization hasn't been started yet. The commit message should say what the effect of this change is to the user. Is this a bugfix? Will it have any effect on runtime at all? To me, hearing your thoughts on this is valuable even if you have to guess. "I noticed this mistake during review and I don't think it will affect runtime." regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpa: fix error code in vp_vdpa_probe()
Return -ENOMEM if vp_modern_map_vq_notify() fails. Currently it
returns success.
Fixes: 11d8ffed00b2 ("vp_vdpa: switch to use vp_modern_map_vq_notify()")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/virtio_pci/vp_vdpa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
b/drivers/vdpa/virtio_pci/vp_vdpa.c
index c76ebb531212..e5d92db728d3 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -442,6 +442,7 @@ static int vp_vdpa_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
vp_modern_map_vq_notify(mdev, i,
_vdpa->vring[i].notify_pa);
if (!vp_vdpa->vring[i].notify) {
+ ret = -ENOMEM;
dev_warn(>dev, "Fail to map vq notify %d\n", i);
goto err;
}
--
2.30.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v7 04/12] virtio-blk: Add validation for block size in config space
On Wed, May 19, 2021 at 09:39:20PM +0800, Yongji Xie wrote: > On Mon, May 17, 2021 at 5:56 PM Xie Yongji wrote: > > > > This ensures that we will not use an invalid block size > > in config space (might come from an untrusted device). I looked at if I should add this as an untrusted function so that Smatch could find these sorts of bugs but this is reading data from the host so there has to be some level of trust... I should add some more untrusted data kvm functions to Smatch. Right now I only have kvm_register_read() and I've added kvm_read_guest_virt() just now. > > > > Signed-off-by: Xie Yongji > > --- > > drivers/block/virtio_blk.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > > index ebb4d3fe803f..c848aa36d49b 100644 > > --- a/drivers/block/virtio_blk.c > > +++ b/drivers/block/virtio_blk.c > > @@ -826,7 +826,7 @@ static int virtblk_probe(struct virtio_device *vdev) > > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, > >struct virtio_blk_config, blk_size, > >_size); > > - if (!err) > > + if (!err && blk_size > 0 && blk_size <= max_size) > > The check here is incorrect. I will use PAGE_SIZE as the maximum > boundary in the new version. What does this bug look like to the user? A minimum block size of 1 seems pretty crazy. Surely the minimum should be higher? regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v6 01/10] file: Export receive_fd() to modules
On Wed, Mar 31, 2021 at 11:15:45AM +0200, Christian Brauner wrote:
> On Wed, Mar 31, 2021 at 04:05:10PM +0800, Xie Yongji wrote:
> > Export receive_fd() so that some modules can use
> > it to pass file descriptor between processes without
> > missing any security stuffs.
> >
> > Signed-off-by: Xie Yongji
> > ---
>
> Yeah, as I said in the other mail I'd be comfortable with exposing just
> this variant of the helper.
> Maybe this should be a separate patch bundled together with Christoph's
> patch to split parts of receive_fd() into a separate helper.
> This would also allow us to simplify a few other codepaths in drivers as
> well btw. I just took a hasty stab at two of them:
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c119736ca56a..3c716bf6d84b 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -3728,8 +3728,9 @@ static int binder_apply_fd_fixups(struct binder_proc
> *proc,
> int ret = 0;
>
> list_for_each_entry(fixup, >fd_fixups, fixup_entry) {
> - int fd = get_unused_fd_flags(O_CLOEXEC);
> + int fd = receive_fd(fixup->file, O_CLOEXEC);
^^^
Assignment duplicated on the next line.
>
> + fd = receive_fd(fixup->file, O_CLOEXEC);
> if (fd < 0) {
> binder_debug(BINDER_DEBUG_TRANSACTION,
> "failed fd fixup txn %d fd %d\n",
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vdpa: set the virtqueue num during register
Hello Jason Wang,
The patch ddd50f4495d3: "vdpa: set the virtqueue num during register"
from Feb 23, 2021, leads to the following static checker warning:
drivers/vdpa/ifcvf/ifcvf_main.c:433 ifcvf_probe()
warn: risky error pointer math: '__vdpa_alloc_device(dev,
_vdpa_ops, 2592 + (0), (0)))'
include/linux/vdpa.h
255 #define vdpa_alloc_device(dev_struct, member, parent, config, name) \
256container_of(__vdpa_alloc_device( \
257 parent, config, \
258 sizeof(dev_struct) + \
259 BUILD_BUG_ON_ZERO(offsetof( \
260 dev_struct, member)), name), \
261 dev_struct, member)
262
The __vdpa_alloc_device() returns an error pointer and if we call
container_of() on then that's a bug... (Unless the container_of() is
known to be a no-op, in which case it's sort of ugly but fine, I guess.
There is one caller where this is the case.).
drivers/vdpa/ifcvf/ifcvf_main.c
432
433 adapter = vdpa_alloc_device(struct ifcvf_adapter, vdpa,
434 dev, _vdpa_ops, NULL);
435 if (adapter == NULL) {
All the other caller check for NULL. :P
436 IFCVF_ERR(pdev, "Failed to allocate vDPA structure");
437 return -ENOMEM;
438 }
439
440 pci_set_master(pdev);
441 pci_set_drvdata(pdev, adapter);
442
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] vdpa: introduce virtio pci driver
Hello Jason Wang,
The patch 010eee82c84e: "vdpa: introduce virtio pci driver" from Feb
23, 2021, leads to the following static checker warning:
drivers/vdpa/virtio_pci/vp_vdpa.c:168 vp_vdpa_request_irq()
warn: inconsistent indenting
drivers/vdpa/virtio_pci/vp_vdpa.c
154 goto err;
155 }
156 vp_modern_queue_vector(mdev, i, i);
157 vp_vdpa->vring[i].irq = irq;
158 }
159
160 snprintf(vp_vdpa->msix_name, VP_VDPA_NAME_SIZE,
"vp-vdpa[%s]-config\n",
161 pci_name(pdev));
162 irq = pci_irq_vector(pdev, queues);
163 ret = devm_request_irq(>dev, irq, vp_vdpa_config_handler,
0,
164 vp_vdpa->msix_name, vp_vdpa);
165 if (ret) {
166 dev_err(>dev,
167 "vp_vdpa: fail to request irq for vq %d\n", i);
Is this error message right?
168 goto err;
indented too far
169 }
170 vp_modern_config_vector(mdev, queues);
171 vp_vdpa->config_irq = irq;
172
173 return 0;
174 err:
175 vp_vdpa_free_irq(vp_vdpa);
176 return ret;
177 }
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vhost_vdpa: return -EFAULT if copy_to_user() fails
The copy_to_user() function returns the number of bytes remaining to be
copied but this should return -EFAULT to the user.
Fixes: 1b48dc03e575 ("vhost: vdpa: report iova range")
Signed-off-by: Dan Carpenter
---
drivers/vhost/vdpa.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index d6a37b66770b..ef688c8c0e0e 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -344,7 +344,9 @@ static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v,
u32 __user *argp)
.last = v->range.last,
};
- return copy_to_user(argp, , sizeof(range));
+ if (copy_to_user(argp, , sizeof(range)))
+ return -EFAULT;
+ return 0;
}
static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
--
2.29.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Sun, Nov 22, 2020 at 08:17:03AM -0800, Kees Cook wrote: > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote: > > On Fri, 20 Nov 2020 11:30:40 -0800 Kees Cook wrote: > > > On Fri, Nov 20, 2020 at 10:53:44AM -0800, Jakub Kicinski wrote: > > > > On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote: > > > > > This series aims to fix almost all remaining fall-through warnings in > > > > > order to enable -Wimplicit-fallthrough for Clang. > > > > > > > > > > In preparation to enable -Wimplicit-fallthrough for Clang, explicitly > > > > > add multiple break/goto/return/fallthrough statements instead of just > > > > > letting the code fall through to the next case. > > > > > > > > > > Notice that in order to enable -Wimplicit-fallthrough for Clang, this > > > > > change[1] is meant to be reverted at some point. So, this patch helps > > > > > to move in that direction. > > > > > > > > > > Something important to mention is that there is currently a > > > > > discrepancy > > > > > between GCC and Clang when dealing with switch fall-through to empty > > > > > case > > > > > statements or to cases that only contain a break/continue/return > > > > > statement[2][3][4]. > > > > > > > > Are we sure we want to make this change? Was it discussed before? > > > > > > > > Are there any bugs Clangs puritanical definition of fallthrough helped > > > > find? > > > > > > > > IMVHO compiler warnings are supposed to warn about issues that could > > > > be bugs. Falling through to default: break; can hardly be a bug?! > > > > > > It's certainly a place where the intent is not always clear. I think > > > this makes all the cases unambiguous, and doesn't impact the machine > > > code, since the compiler will happily optimize away any behavioral > > > redundancy. > > > > If none of the 140 patches here fix a real bug, and there is no change > > to machine code then it sounds to me like a W=2 kind of a warning. > > FWIW, this series has found at least one bug so far: > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/ This is a fallthrough to a return and not to a break. That should trigger a warning. The fallthrough to a break should not generate a warning. The bug we're trying to fix is "missing break statement" but if the result of the bug is "we hit a break statement" then now we're just talking about style. GCC should limit itself to warning about potentially buggy code. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, Nov 23, 2020 at 05:32:51PM -0800, Nick Desaulniers wrote:
> On Sun, Nov 22, 2020 at 8:17 AM Kees Cook wrote:
> >
> > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> > > If none of the 140 patches here fix a real bug, and there is no change
> > > to machine code then it sounds to me like a W=2 kind of a warning.
> >
> > FWIW, this series has found at least one bug so far:
> > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
>
> So looks like the bulk of these are:
> switch (x) {
> case 0:
> ++x;
> default:
> break;
> }
This should not generate a warning.
>
> I have a patch that fixes those up for clang:
> https://reviews.llvm.org/D91895
>
> There's 3 other cases that don't quite match between GCC and Clang I
> observe in the kernel:
> switch (x) {
> case 0:
> ++x;
> default:
> goto y;
> }
> y:;
This should generate a warning.
>
> switch (x) {
> case 0:
> ++x;
> default:
> return;
> }
Warn for this.
>
> switch (x) {
> case 0:
> ++x;
> default:
> ;
> }
Don't warn for this.
If adding a break statement changes the flow of the code then warn about
potentially missing break statements, but if it doesn't change anything
then don't warn about it.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/17] vhost: prep vhost_dev_init users to handle failures
Hi Mike,
url:
https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-cgroup-support/20201022-083844
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-m021-20201101 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vsock.c:648 vhost_vsock_dev_open() error: uninitialized symbol
'ret'.
vim +/ret +648 drivers/vhost/vsock.c
433fc58e6bf2c8b Asias He2016-07-28 605 static int
vhost_vsock_dev_open(struct inode *inode, struct file *file)
433fc58e6bf2c8b Asias He2016-07-28 606 {
433fc58e6bf2c8b Asias He2016-07-28 607 struct vhost_virtqueue
**vqs;
433fc58e6bf2c8b Asias He2016-07-28 608 struct vhost_vsock
*vsock;
433fc58e6bf2c8b Asias He2016-07-28 609 int ret;
433fc58e6bf2c8b Asias He2016-07-28 610
433fc58e6bf2c8b Asias He2016-07-28 611 /* This struct is large
and allocation could fail, fall back to vmalloc
433fc58e6bf2c8b Asias He2016-07-28 612 * if there is no other
way.
433fc58e6bf2c8b Asias He2016-07-28 613 */
dcda9b04713c3f6 Michal Hocko2017-07-12 614 vsock =
kvmalloc(sizeof(*vsock), GFP_KERNEL | __GFP_RETRY_MAYFAIL);
433fc58e6bf2c8b Asias He2016-07-28 615 if (!vsock)
433fc58e6bf2c8b Asias He2016-07-28 616 return -ENOMEM;
433fc58e6bf2c8b Asias He2016-07-28 617
433fc58e6bf2c8b Asias He2016-07-28 618 vqs =
kmalloc_array(ARRAY_SIZE(vsock->vqs), sizeof(*vqs), GFP_KERNEL);
433fc58e6bf2c8b Asias He2016-07-28 619 if (!vqs) {
433fc58e6bf2c8b Asias He2016-07-28 620 ret = -ENOMEM;
433fc58e6bf2c8b Asias He2016-07-28 621 goto out;
433fc58e6bf2c8b Asias He2016-07-28 622 }
433fc58e6bf2c8b Asias He2016-07-28 623
a72b69dc083a931 Stefan Hajnoczi 2017-11-09 624 vsock->guest_cid = 0;
/* no CID assigned yet */
a72b69dc083a931 Stefan Hajnoczi 2017-11-09 625
433fc58e6bf2c8b Asias He2016-07-28 626
atomic_set(>queued_replies, 0);
433fc58e6bf2c8b Asias He2016-07-28 627
433fc58e6bf2c8b Asias He2016-07-28 628 vqs[VSOCK_VQ_TX] =
>vqs[VSOCK_VQ_TX];
433fc58e6bf2c8b Asias He2016-07-28 629 vqs[VSOCK_VQ_RX] =
>vqs[VSOCK_VQ_RX];
433fc58e6bf2c8b Asias He2016-07-28 630
vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick;
433fc58e6bf2c8b Asias He2016-07-28 631
vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick;
433fc58e6bf2c8b Asias He2016-07-28 632
6e1629548d318c2 Mike Christie 2020-10-21 633 if
(vhost_dev_init(>dev, vqs, ARRAY_SIZE(vsock->vqs),
e82b9b0727ff6d6 Jason Wang 2019-05-17 634
UIO_MAXIOV, VHOST_VSOCK_PKT_WEIGHT,
6e1629548d318c2 Mike Christie 2020-10-21 635
VHOST_VSOCK_WEIGHT, true, NULL))
6e1629548d318c2 Mike Christie 2020-10-21 636 goto
err_dev_init;
^
"ret" needs to be set here.
433fc58e6bf2c8b Asias He2016-07-28 637
433fc58e6bf2c8b Asias He2016-07-28 638 file->private_data =
vsock;
433fc58e6bf2c8b Asias He2016-07-28 639
spin_lock_init(>send_pkt_list_lock);
433fc58e6bf2c8b Asias He2016-07-28 640
INIT_LIST_HEAD(>send_pkt_list);
433fc58e6bf2c8b Asias He2016-07-28 641
vhost_work_init(>send_pkt_work, vhost_transport_send_pkt_work);
433fc58e6bf2c8b Asias He2016-07-28 642 return 0;
433fc58e6bf2c8b Asias He2016-07-28 643
6e1629548d318c2 Mike Christie 2020-10-21 644 err_dev_init:
6e1629548d318c2 Mike Christie 2020-10-21 645 kfree(vqs);
433fc58e6bf2c8b Asias He2016-07-28 646 out:
433fc58e6bf2c8b Asias He2016-07-28 647 vhost_vsock_free(vsock);
433fc58e6bf2c8b Asias He2016-07-28 @648 return ret;
433fc58e6bf2c8b Asias He2016-07-28 649 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: Fix a double free in virtio_gpu_cmd_map()
This is freed both here and in the caller (virtio_gpu_vram_map()) so
it's a double free. The correct place is only in the caller.
Fixes: 16845c5d5409 ("drm/virtio: implement blob resources: implement vram
object")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 72586cd8cc4c..3f200306c9d7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -1212,10 +1212,8 @@ int virtio_gpu_cmd_map(struct virtio_gpu_device *vgdev,
struct virtio_gpu_resp_map_info *resp_buf;
resp_buf = kzalloc(sizeof(*resp_buf), GFP_KERNEL);
- if (!resp_buf) {
- virtio_gpu_array_put_free(objs);
+ if (!resp_buf)
return -ENOMEM;
- }
cmd_p = virtio_gpu_alloc_cmd_resp
(vgdev, virtio_gpu_cmd_resource_map_cb, , sizeof(*cmd_p),
--
2.28.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH net] vhost_vdpa: Return -EFUALT if copy_from_user() fails
The copy_to/from_user() functions return the number of bytes which we
weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes: a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by: Dan Carpenter
---
drivers/vhost/vdpa.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 62a9bb0efc55..c94a97b6bd6d 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -428,12 +428,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
void __user *argp = (void __user *)arg;
u64 __user *featurep = argp;
u64 features;
- long r;
+ long r = 0;
if (cmd == VHOST_SET_BACKEND_FEATURES) {
- r = copy_from_user(, featurep, sizeof(features));
- if (r)
- return r;
+ if (copy_from_user(, featurep, sizeof(features)))
+ return -EFAULT;
if (features & ~VHOST_VDPA_BACKEND_FEATURES)
return -EOPNOTSUPP;
vhost_set_backend_features(>vdev, features);
@@ -476,7 +475,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
break;
case VHOST_GET_BACKEND_FEATURES:
features = VHOST_VDPA_BACKEND_FEATURES;
- r = copy_to_user(featurep, , sizeof(features));
+ if (copy_to_user(featurep, , sizeof(features)))
+ r = -EFAULT;
break;
default:
r = vhost_dev_ioctl(>vdev, cmd, argp);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/16] vhost: prep vhost_dev_init users to handle failures
Hi Mike,
url:
https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m001-20201008 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/vhost/vdpa.c:844 vhost_vdpa_open() error: uninitialized symbol 'r'.
Old smatch warnings:
drivers/vhost/vdpa.c:436 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT
instead of the bytes remaining?
drivers/vhost/vdpa.c:489 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT
instead of the bytes remaining?
vim +/r +844 drivers/vhost/vdpa.c
4c8cf31885f69e8 Tiwei Bie 2020-03-26 793 static int
vhost_vdpa_open(struct inode *inode, struct file *filep)
4c8cf31885f69e8 Tiwei Bie 2020-03-26 794 {
4c8cf31885f69e8 Tiwei Bie 2020-03-26 795 struct vhost_vdpa *v;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 796 struct vhost_dev *dev;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 797 struct vhost_virtqueue **vqs;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 798 int nvqs, i, r, opened;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 799
4c8cf31885f69e8 Tiwei Bie 2020-03-26 800 v = container_of(inode->i_cdev,
struct vhost_vdpa, cdev);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 801
4c8cf31885f69e8 Tiwei Bie 2020-03-26 802 opened =
atomic_cmpxchg(>opened, 0, 1);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 803 if (opened)
4c8cf31885f69e8 Tiwei Bie 2020-03-26 804 return -EBUSY;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 805
4c8cf31885f69e8 Tiwei Bie 2020-03-26 806 nvqs = v->nvqs;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 807 vhost_vdpa_reset(v);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 808
4c8cf31885f69e8 Tiwei Bie 2020-03-26 809 vqs = kmalloc_array(nvqs,
sizeof(*vqs), GFP_KERNEL);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 810 if (!vqs) {
4c8cf31885f69e8 Tiwei Bie 2020-03-26 811 r = -ENOMEM;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 812 goto err;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 813 }
4c8cf31885f69e8 Tiwei Bie 2020-03-26 814
4c8cf31885f69e8 Tiwei Bie 2020-03-26 815 dev = >vdev;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 816 for (i = 0; i < nvqs; i++) {
4c8cf31885f69e8 Tiwei Bie 2020-03-26 817 vqs[i] = >vqs[i];
4c8cf31885f69e8 Tiwei Bie 2020-03-26 818 vqs[i]->handle_kick =
handle_vq_kick;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 819 }
7dc4d1082d406f3 Mike Christie 2020-10-07 820 if (vhost_dev_init(dev, vqs,
nvqs, 0, 0, 0, false,
7dc4d1082d406f3 Mike Christie 2020-10-07 821
vhost_vdpa_process_iotlb_msg))
7dc4d1082d406f3 Mike Christie 2020-10-07 822 goto err_dev_init;
"r" not set on this error path.
4c8cf31885f69e8 Tiwei Bie 2020-03-26 823
4c8cf31885f69e8 Tiwei Bie 2020-03-26 824 dev->iotlb =
vhost_iotlb_alloc(0, 0);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 825 if (!dev->iotlb) {
4c8cf31885f69e8 Tiwei Bie 2020-03-26 826 r = -ENOMEM;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 827 goto err_init_iotlb;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 828 }
4c8cf31885f69e8 Tiwei Bie 2020-03-26 829
4c8cf31885f69e8 Tiwei Bie 2020-03-26 830 r = vhost_vdpa_alloc_domain(v);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 831 if (r)
4c8cf31885f69e8 Tiwei Bie 2020-03-26 832 goto err_init_iotlb;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 833
4c8cf31885f69e8 Tiwei Bie 2020-03-26 834 filep->private_data = v;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 835
4c8cf31885f69e8 Tiwei Bie 2020-03-26 836 return 0;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 837
4c8cf31885f69e8 Tiwei Bie 2020-03-26 838 err_init_iotlb:
4c8cf31885f69e8 Tiwei Bie 2020-03-26 839 vhost_dev_cleanup(>vdev);
7dc4d1082d406f3 Mike Christie 2020-10-07 840 err_dev_init:
37787e9f81e2e58 Mike Christie 2020-09-21 841 kfree(vqs);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 842 err:
4c8cf31885f69e8 Tiwei Bie 2020-03-26 843 atomic_dec(>opened);
4c8cf31885f69e8 Tiwei Bie 2020-03-26 @844 return r;
4c8cf31885f69e8 Tiwei Bie 2020-03-26 845 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/8] vhost scsi: alloc cmds per vq instead of session
Hi Mike,
url:
https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-m021-20200923 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/scsi.c:606 vhost_scsi_get_cmd() error: uninitialized symbol 'cpu'.
#
https://github.com/0day-ci/linux/commit/aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Mike-Christie/vhost-scsi-fixes-and-cleanups/20200922-031251
git checkout aef0e1e9298ab68f2d7bdf1afb9a376641b993d5
vim +/cpu +606 drivers/vhost/scsi.c
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 572
static struct vhost_scsi_cmd *
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 573
vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 574
unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr,
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 575
u32 exp_data_len, int data_direction)
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 576 {
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 577
struct vhost_scsi_virtqueue *svq = container_of(vq,
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 578
struct vhost_scsi_virtqueue, vq);
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 579
struct vhost_scsi_cmd *cmd;
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 580
struct vhost_scsi_nexus *tv_nexus;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 581
struct scatterlist *sg, *prot_sg;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 582
struct page **pages;
10e9cbb6b531117b drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 583
int tag, cpu;
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 584
9871831283e79575 drivers/vhost/scsi.c Asias He 2013-05-06 585
tv_nexus = tpg->tpg_nexus;
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 586
if (!tv_nexus) {
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 587
pr_err("Unable to locate active struct vhost_scsi_nexus\n");
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 588
return ERR_PTR(-EIO);
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 589
}
057cbf49a1f08297 drivers/vhost/tcm_vhost.c Nicholas Bellinger 2012-07-18 590
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 591
tag = sbitmap_get(>scsi_tags, 0, false);
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 592
if (tag < 0) {
1a1ff8256af679c8 drivers/vhost/scsi.c Nicholas Bellinger 2015-01-31 593
pr_err("Unable to obtain tag for vhost_scsi_cmd\n");
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 594
return ERR_PTR(-ENOMEM);
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 595
}
4a47d3a1ff10e564 drivers/vhost/scsi.c Nicholas Bellinger 2013-09-23 596
aef0e1e9298ab68f drivers/vhost/scsi.c Mike Christie 2020-09-21 597
cmd = >scsi_cmds[tag];
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 598
sg = cmd->tvc_sgl;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 599
prot_sg = cmd->tvc_prot_sgl;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 600
pages = cmd->tvc_upages;
473f0b15a4c97d39 drivers/vhost/scsi.c Markus Elfring 2017-05-20 601
memset(cmd, 0, sizeof(*cmd));
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 602
cmd->tvc_sgl = sg;
b1935f687bb93b20 drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 603
cmd->tvc_prot_sgl = prot_sg;
3aee26b4ae91048c drivers/vhost/scsi.c Nicholas Bellinger 2013-06-21 604
cmd->tvc_upages = pages;
4824d3bfb9097ac5 drivers/vhost/scsi.c Nicholas Bellinger 2013-06-07 605
cmd->tvc_se_cmd.map_tag = tag;
10e9cbb6b531117b drivers/vhost/scsi.c Matthew Wilcox 2018-06-12 @606
cmd->tvc_se_cmd.map_cpu = cpu;
"cpu" is never initialized.
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02-22 607
cmd->tvc_tag = scsi_tag;
95e7c4341b8e28da drivers/vhost/scsi.c Nicholas Bellinger 2014-02
Re: [PATCH v2] i2c: virtio: add a virtio i2c frontend driver
On Mon, Sep 14, 2020 at 06:24:55PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 14, 2020 at 05:48:07PM +0300, Dan Carpenter wrote:
> > Hi Jie,
> >
> > url:
> > https://github.com/0day-ci/linux/commits/Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20200911-115013
> >
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
> > i2c/for-next
> > config: parisc-randconfig-m031-20200913 (attached as .config)
> > compiler: hppa-linux-gcc (GCC) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot
> > Reported-by: Dan Carpenter
> >
> > smatch warnings:
> > drivers/i2c/busses/i2c-virtio.c:160 virtio_i2c_xfer() error: we previously
> > assumed 'vmsg' could be null (see line 137)
> >
>
> It's quite possible a false positive. Look at 122. But I agree that for-loop
> is
> not the best for such things to understand. Perhaps switching to do {} while
> ()
> will make it better.
>
Smatch is assuming that virtqueue_get_buf() can return NULL on the last
iteration through the loop.
regards,
dan carpenter
> > #
> > https://github.com/0day-ci/linux/commit/0a54ec771966748fcbc86256b830b5f786168b7d
> >
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review
> > Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20200911-115013
> > git checkout 0a54ec771966748fcbc86256b830b5f786168b7d
> > vim +/vmsg +160 drivers/i2c/busses/i2c-virtio.c
> >
> > 0a54ec77196674 Jie Deng 2020-09-11 109 static int virtio_i2c_xfer(struct
> > i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > 0a54ec77196674 Jie Deng 2020-09-11 110 {
> > 0a54ec77196674 Jie Deng 2020-09-11 111 struct virtio_i2c *vi =
> > i2c_get_adapdata(adap);
> > 0a54ec77196674 Jie Deng 2020-09-11 112 struct virtqueue *vq = vi->vq;
> > 0a54ec77196674 Jie Deng 2020-09-11 113 struct virtio_i2c_msg *vmsg;
> > 0a54ec77196674 Jie Deng 2020-09-11 114 unsigned long time_left;
> > 0a54ec77196674 Jie Deng 2020-09-11 115 int len, i, ret = 0;
> > 0a54ec77196674 Jie Deng 2020-09-11 116
> > 0a54ec77196674 Jie Deng 2020-09-11 117 mutex_lock(>i2c_lock);
> > 0a54ec77196674 Jie Deng 2020-09-11 118 vmsg = >vmsg;
> > 0a54ec77196674 Jie Deng 2020-09-11 119 vmsg->buf = NULL;
> > 0a54ec77196674 Jie Deng 2020-09-11 120
> > 0a54ec77196674 Jie Deng 2020-09-11 121 for (i = 0; i < num; i++) {
> > 0a54ec77196674 Jie Deng 2020-09-11 122 ret =
> > virtio_i2c_add_msg(vq, vmsg, [i]);
> > 0a54ec77196674 Jie Deng 2020-09-11 123 if (ret) {
> > 0a54ec77196674 Jie Deng 2020-09-11 124
> > dev_err(>dev, "failed to add msg[%d] to virtqueue.\n", i);
> > 0a54ec77196674 Jie Deng 2020-09-11 125 break;
> > 0a54ec77196674 Jie Deng 2020-09-11 126 }
> > 0a54ec77196674 Jie Deng 2020-09-11 127
> > 0a54ec77196674 Jie Deng 2020-09-11 128 virtqueue_kick(vq);
> > 0a54ec77196674 Jie Deng 2020-09-11 129
> > 0a54ec77196674 Jie Deng 2020-09-11 130 time_left =
> > wait_for_completion_timeout(>completion, adap->timeout);
> > 0a54ec77196674 Jie Deng 2020-09-11 131 if (!time_left) {
> > 0a54ec77196674 Jie Deng 2020-09-11 132
> > dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, msgs[i].addr);
> > 0a54ec77196674 Jie Deng 2020-09-11 133 break;
> > 0a54ec77196674 Jie Deng 2020-09-11 134 }
> > 0a54ec77196674 Jie Deng 2020-09-11 135
> > 0a54ec77196674 Jie Deng 2020-09-11 136 vmsg = (struct
> > virtio_i2c_msg *)virtqueue_get_buf(vq, );
> > 0a54ec77196674 Jie Deng 2020-09-11 @137 if (vmsg) {
> >
> > Check for NULL.
> >
> > 0a54ec77196674 Jie Deng 2020-09-11 138 /* vmsg should
> > point to the same address with >vmsg */
> > 0a54ec77196674 Jie Deng 2020-09-11 139 if (vmsg !=
> > >vmsg) {
> > 0a54ec77196674 Jie Deng 2020-09-11 140
> > dev_err(>dev, "msg[%d]: addr=0x%x virtqueue error.\n",
> > 0a54ec77196674 Jie Deng 2020-09-11 141
> > i, le16_to_cpu(vmsg->hdr.addr));
> > 0a54ec77196674 Jie Deng 2020-09-11 142 break;
> > 0a54ec77196674 Jie Deng 2020-09-11 143 }
> &
Re: [PATCH v2] i2c: virtio: add a virtio i2c frontend driver
Hi Jie,
url:
https://github.com/0day-ci/linux/commits/Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20200911-115013
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git
i2c/for-next
config: parisc-randconfig-m031-20200913 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/i2c/busses/i2c-virtio.c:160 virtio_i2c_xfer() error: we previously
assumed 'vmsg' could be null (see line 137)
#
https://github.com/0day-ci/linux/commit/0a54ec771966748fcbc86256b830b5f786168b7d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jie-Deng/i2c-virtio-add-a-virtio-i2c-frontend-driver/20200911-115013
git checkout 0a54ec771966748fcbc86256b830b5f786168b7d
vim +/vmsg +160 drivers/i2c/busses/i2c-virtio.c
0a54ec77196674 Jie Deng 2020-09-11 109 static int virtio_i2c_xfer(struct
i2c_adapter *adap, struct i2c_msg *msgs, int num)
0a54ec77196674 Jie Deng 2020-09-11 110 {
0a54ec77196674 Jie Deng 2020-09-11 111 struct virtio_i2c *vi =
i2c_get_adapdata(adap);
0a54ec77196674 Jie Deng 2020-09-11 112 struct virtqueue *vq = vi->vq;
0a54ec77196674 Jie Deng 2020-09-11 113 struct virtio_i2c_msg *vmsg;
0a54ec77196674 Jie Deng 2020-09-11 114 unsigned long time_left;
0a54ec77196674 Jie Deng 2020-09-11 115 int len, i, ret = 0;
0a54ec77196674 Jie Deng 2020-09-11 116
0a54ec77196674 Jie Deng 2020-09-11 117 mutex_lock(>i2c_lock);
0a54ec77196674 Jie Deng 2020-09-11 118 vmsg = >vmsg;
0a54ec77196674 Jie Deng 2020-09-11 119 vmsg->buf = NULL;
0a54ec77196674 Jie Deng 2020-09-11 120
0a54ec77196674 Jie Deng 2020-09-11 121 for (i = 0; i < num; i++) {
0a54ec77196674 Jie Deng 2020-09-11 122 ret =
virtio_i2c_add_msg(vq, vmsg, [i]);
0a54ec77196674 Jie Deng 2020-09-11 123 if (ret) {
0a54ec77196674 Jie Deng 2020-09-11 124
dev_err(>dev, "failed to add msg[%d] to virtqueue.\n", i);
0a54ec77196674 Jie Deng 2020-09-11 125 break;
0a54ec77196674 Jie Deng 2020-09-11 126 }
0a54ec77196674 Jie Deng 2020-09-11 127
0a54ec77196674 Jie Deng 2020-09-11 128 virtqueue_kick(vq);
0a54ec77196674 Jie Deng 2020-09-11 129
0a54ec77196674 Jie Deng 2020-09-11 130 time_left =
wait_for_completion_timeout(>completion, adap->timeout);
0a54ec77196674 Jie Deng 2020-09-11 131 if (!time_left) {
0a54ec77196674 Jie Deng 2020-09-11 132
dev_err(>dev, "msg[%d]: addr=0x%x timeout.\n", i, msgs[i].addr);
0a54ec77196674 Jie Deng 2020-09-11 133 break;
0a54ec77196674 Jie Deng 2020-09-11 134 }
0a54ec77196674 Jie Deng 2020-09-11 135
0a54ec77196674 Jie Deng 2020-09-11 136 vmsg = (struct
virtio_i2c_msg *)virtqueue_get_buf(vq, );
0a54ec77196674 Jie Deng 2020-09-11 @137 if (vmsg) {
Check for NULL.
0a54ec77196674 Jie Deng 2020-09-11 138 /* vmsg should
point to the same address with >vmsg */
0a54ec77196674 Jie Deng 2020-09-11 139 if (vmsg !=
>vmsg) {
0a54ec77196674 Jie Deng 2020-09-11 140
dev_err(>dev, "msg[%d]: addr=0x%x virtqueue error.\n",
0a54ec77196674 Jie Deng 2020-09-11 141
i, le16_to_cpu(vmsg->hdr.addr));
0a54ec77196674 Jie Deng 2020-09-11 142 break;
0a54ec77196674 Jie Deng 2020-09-11 143 }
0a54ec77196674 Jie Deng 2020-09-11 144 if
(vmsg->status != VIRTIO_I2C_MSG_OK) {
0a54ec77196674 Jie Deng 2020-09-11 145
dev_err(>dev, "msg[%d]: addr=0x%x error=%d.\n",
0a54ec77196674 Jie Deng 2020-09-11 146
i, le16_to_cpu(vmsg->hdr.addr), vmsg->status);
0a54ec77196674 Jie Deng 2020-09-11 147 break;
0a54ec77196674 Jie Deng 2020-09-11 148 }
0a54ec77196674 Jie Deng 2020-09-11 149 if
((msgs[i].flags & I2C_M_RD) && msgs[i].len)
0a54ec77196674 Jie Deng 2020-09-11 150
memcpy(msgs[i].buf, vmsg->buf, msgs[i].len);
0a54ec77196674 Jie Deng 2020-09-11 151
0a54ec77196674 Jie Deng 2020-09-11 152
kfree(vmsg->buf);
0a54ec77196674 Jie Deng 2020-09-11 153 vmsg->buf =
NULL;
0a54ec77196674 Jie Deng 2020-09-11 154 }
0a54ec77196674 Jie Deng 2020-09-11 155
0a54ec77196674 Jie Deng 2020-09-11 156
rein
Re: [PATCH] vhost: new vhost_vdpa SET/GET_BACKEND_FEATURES handlers
Hi Zhu,
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vhost-new-vhost_vdpa-SET-GET_BACKEND_FEATURES-handlers/20200909-115726
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: parisc-randconfig-m031-20200909 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/vhost/vdpa.c:356 vhost_vdpa_get_backend_features() warn: maybe return
-EFAULT instead of the bytes remaining?
#
https://github.com/0day-ci/linux/commit/f2da8fc35b4ef003de7d559a8c7730fedf9926f8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Zhu-Lingshan/vhost-new-vhost_vdpa-SET-GET_BACKEND_FEATURES-handlers/20200909-115726
git checkout f2da8fc35b4ef003de7d559a8c7730fedf9926f8
vim +356 drivers/vhost/vdpa.c
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 348 static long
vhost_vdpa_get_backend_features(void __user *argp)
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 349 {
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 350 u64 features =
VHOST_VDPA_BACKEND_FEATURES;
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 351 u64 __user *featurep = argp;
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 352 long r;
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 353
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 354 r = copy_to_user(featurep,
, sizeof(features));
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 355
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 @356 return r;
copy_to_user() returns the number of bytes remaining. I haven't looked
at how this is called but it should probably return negative error codes
on error:
if (copy_to_user(featurep, , sizeof(features)))
return -EFAULT;
return 0;
f2da8fc35b4ef0 Zhu Lingshan 2020-09-09 357 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa/mlx5: Fix pointer math in mlx5_vdpa_get_config()
On Sun, Aug 09, 2020 at 06:34:04AM +, Eli Cohen wrote: > Acked-by: Eli Cohen > > BTW, vdpa_sim has the same bug. > I sent a patch for that on April 6. [PATCH 2/2] vdpa: Fix pointer math bug in vdpasim_get_config() Jason acked the patch but it wasn't applied. regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpa/mlx5: Fix pointer math in mlx5_vdpa_get_config()
There is a pointer math bug here so if "offset" is non-zero then this
will copy memory from beyond the end of the array.
Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 3ec44a4f0e45..9d1637cf772e 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1758,7 +1758,7 @@ static void mlx5_vdpa_get_config(struct vdpa_device
*vdev, unsigned int offset,
struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
if (offset + len < sizeof(struct virtio_net_config))
- memcpy(buf, >config + offset, len);
+ memcpy(buf, (u8 *)>config + offset, len);
}
static void mlx5_vdpa_set_config(struct vdpa_device *vdev, unsigned int
offset, const void *buf,
--
2.27.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vdpa/mlx5: Missing error code on allocation failure
This should return -ENOMEM if the allocation fails. Currently it
either returns success or an uninitialized value.
Fixes: 94abbccdf291 ("vdpa/mlx5: Add shared memory registration code")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/mlx5/core/mr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index f5dec0274133..ef1c550f8266 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -319,8 +319,10 @@ static int add_direct_chain(struct mlx5_vdpa_dev *mvdev,
u64 start, u64 size, u8
while (size) {
sz = (u32)min_t(u64, MAX_KLM_SIZE, size);
dmr = kzalloc(sizeof(*dmr), GFP_KERNEL);
- if (!dmr)
+ if (!dmr) {
+ err = -ENOMEM;
goto err_alloc;
+ }
dmr->start = st;
dmr->end = st + sz;
--
2.27.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: Fix an IS_ERR() vs NULL check in virtio_gpu_object_shmem_init()
The drm_gem_shmem_get_pages_sgt() function returns error pointers on
error, it never returns NULL.
Fixes: d323bb44e4d2 ("drm/virtio: Call the right shmem helpers")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/virtio/virtgpu_object.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 346cef5ce251..0cd5ecf4b3c0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -151,9 +151,9 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
return -EINVAL;
shmem->pages = drm_gem_shmem_get_pages_sgt(>base.base);
- if (!shmem->pages) {
+ if (IS_ERR(shmem->pages)) {
drm_gem_shmem_unpin(>base.base);
- return -EINVAL;
+ return PTR_ERR(shmem->pages);
}
if (use_dma_api) {
--
2.27.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 2/3] mm, treewide: Rename kzfree() to kfree_sensitive()
Last time you sent this we couldn't decide which tree it should go through. Either the crypto tree or through Andrew seems like the right thing to me. Also the other issue is that it risks breaking things if people add new kzfree() instances while we are doing the transition. Could you just add a "#define kzfree kfree_sensitive" so that things continue to compile and we can remove it in the next kernel release? regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v4 1/3] mm/slab: Use memzero_explicit() in kzfree()
On Tue, Jun 16, 2020 at 08:42:08AM +0200, Michal Hocko wrote:
> On Mon 15-06-20 21:57:16, Waiman Long wrote:
> > The kzfree() function is normally used to clear some sensitive
> > information, like encryption keys, in the buffer before freeing it back
> > to the pool. Memset() is currently used for the buffer clearing. However,
> > it is entirely possible that the compiler may choose to optimize away the
> > memory clearing especially if LTO is being used. To make sure that this
> > optimization will not happen, memzero_explicit(), which is introduced
> > in v3.18, is now used in kzfree() to do the clearing.
> >
> > Fixes: 3ef0e5ba4673 ("slab: introduce kzfree()")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Waiman Long
>
> Acked-by: Michal Hocko
>
> Although I am not really sure this is a stable material. Is there any
> known instance where the memset was optimized out from kzfree?
I told him to add the stable. Otherwise it will just get reported to
me again. It's a just safer to backport it before we forget.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/2] mm, treewide: Rename kzfree() to kfree_sensitive()
On Mon, Apr 13, 2020 at 05:15:49PM -0400, Waiman Long wrote:
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 23c7500eea7d..c08bc7eb20bd 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1707,17 +1707,17 @@ void *krealloc(const void *p, size_t new_size, gfp_t
> flags)
> EXPORT_SYMBOL(krealloc);
>
> /**
> - * kzfree - like kfree but zero memory
> + * kfree_sensitive - Clear sensitive information in memory before freeing
> * @p: object to free memory of
> *
> * The memory of the object @p points to is zeroed before freed.
> - * If @p is %NULL, kzfree() does nothing.
> + * If @p is %NULL, kfree_sensitive() does nothing.
> *
> * Note: this function zeroes the whole allocated buffer which can be a good
> * deal bigger than the requested buffer size passed to kmalloc(). So be
> * careful when using this function in performance sensitive code.
> */
> -void kzfree(const void *p)
> +void kfree_sensitive(const void *p)
> {
> size_t ks;
> void *mem = (void *)p;
> @@ -1725,10 +1725,10 @@ void kzfree(const void *p)
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> ks = ksize(mem);
> - memset(mem, 0, ks);
> + memzero_explicit(mem, ks);
^
This is an unrelated bug fix. It really needs to be pulled into a
separate patch by itself and back ported to stable kernels.
> kfree(mem);
> }
> -EXPORT_SYMBOL(kzfree);
> +EXPORT_SYMBOL(kfree_sensitive);
>
> /**
> * ksize - get the actual amount of memory allocated for a given object
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] vhost_vdpa: Fix potential underflow in vhost_vdpa_mmap()
The "vma->vm_pgoff" variable is an unsigned long so if it's larger than
INT_MAX then "index" can be negative leading to an underflow. Fix this
by changing the type of "index" to "unsigned long".
Fixes: ddd89d0a059d ("vhost_vdpa: support doorbell mapping via mmap")
Signed-off-by: Dan Carpenter
---
drivers/vhost/vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 7580e34f76c10..a54b60d6623f0 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -818,7 +818,7 @@ static int vhost_vdpa_mmap(struct file *file, struct
vm_area_struct *vma)
struct vdpa_device *vdpa = v->vdpa;
const struct vdpa_config_ops *ops = vdpa->config;
struct vdpa_notification_area notify;
- int index = vma->vm_pgoff;
+ unsigned long index = vma->vm_pgoff;
if (vma->vm_end - vma->vm_start != PAGE_SIZE)
return -EINVAL;
--
2.26.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio-mem: silence a static checker warning
Smatch complains that "rc" can be uninitialized if we hit the "break;" statement on the first iteration through the loop. I suspect that this can't happen in real life, but returning a zero literal is cleaner and silence the static checker warning. Signed-off-by: Dan Carpenter --- drivers/virtio/virtio_mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c index f658fe9149beb..893ef18060a02 100644 --- a/drivers/virtio/virtio_mem.c +++ b/drivers/virtio/virtio_mem.c @@ -1192,7 +1192,7 @@ static int virtio_mem_mb_plug_any_sb(struct virtio_mem *vm, unsigned long mb_id, VIRTIO_MEM_MB_STATE_OFFLINE); } - return rc; + return 0; } /* -- 2.26.2 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa: bypass waking up vhost_woker for vdpa vq kick
Hi Zhu,
url:
https://github.com/0day-ci/linux/commits/Zhu-Lingshan/vdpa-bypass-waking-up-vhost_woker-for-vdpa-vq-kick/20200526-133819
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/vhost/vdpa.c:348 vhost_vdpa_set_vring_kick() error: uninitialized
symbol 'r'.
#
https://github.com/0day-ci/linux/commit/a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a84ddbf1ef29f18aafb2bb8a93bcedd4a29a967d
vim +/r +348 drivers/vhost/vdpa.c
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 316 static long
vhost_vdpa_set_vring_kick(struct vhost_virtqueue *vq,
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 317
void __user *argp)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 318 {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 319 bool pollstart = false,
pollstop = false;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 320 struct file *eventfp, *filep =
NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 321 struct vhost_vring_file f;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 322 long r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 323
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 324 if (copy_from_user(, argp,
sizeof(f)))
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 325 return -EFAULT;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 326
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 327 eventfp = f.fd == -1 ? NULL :
eventfd_fget(f.fd);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 328 if (IS_ERR(eventfp)) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 329 r = PTR_ERR(eventfp);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 330 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 331 }
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 332
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 333 if (eventfp != vq->kick) {
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 334 pollstop = (filep =
vq->kick) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 335 pollstart = (vq->kick =
eventfp) != NULL;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 336 } else
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 337 filep = eventfp;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 338
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 339 if (pollstop && vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 340
vhost_vdpa_poll_stop(vq);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 341
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 342 if (filep)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 343 fput(filep);
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 344
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 345 if (pollstart &&
vq->handle_kick)
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 346 r =
vhost_vdpa_poll_start(vq);
"r" not initialized on else path.
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 347
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 @348 return r;
a84ddbf1ef29f1 Zhu Lingshan 2020-05-26 349 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/2] vdpa: Fix pointer math bug in vdpasim_get_config()
If "offset" is non-zero then we end up copying from beyond the end of
the config because of pointer math. We can fix this by casting the
struct to a u8 pointer.
Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Signed-off-by: Dan Carpenter
---
Is it really worth letting people specify the offset?
drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index b3c800653056..e03c25765513 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -509,7 +509,7 @@ static void vdpasim_get_config(struct vdpa_device *vdpa,
unsigned int offset,
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
if (offset + len < sizeof(struct virtio_net_config))
- memcpy(buf, >config + offset, len);
+ memcpy(buf, (u8 *)>config + offset, len);
}
static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/2] vdpa: Signedness bugs in vdpasim_work()
The "read" and "write" variables need to be signed for the error
handling to work.
Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6e8a0cf2fdeb..b3c800653056 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -132,7 +132,8 @@ static void vdpasim_work(struct work_struct *work)
vdpasim, work);
struct vdpasim_virtqueue *txq = >vqs[1];
struct vdpasim_virtqueue *rxq = >vqs[0];
- size_t read, write, total_write;
+ ssize_t read, write;
+ size_t total_write;
int err;
int pkts = 0;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 5/5] vdpasim: vDPA device simulator
Hi Jason,
url:
https://github.com/0day-ci/linux/commits/Jason-Wang/vDPA-support/20200117-170243
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/virtio/vdpa/vdpa_sim.c:288 vdpasim_alloc_coherent() warn: returning
freed memory 'addr'
#
https://github.com/0day-ci/linux/commit/55047769b3e974d68b2aab5ce0022459b172a23f
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 55047769b3e974d68b2aab5ce0022459b172a23f
vim +/addr +288 drivers/virtio/vdpa/vdpa_sim.c
55047769b3e974 Jason Wang 2020-01-16 263 static void
*vdpasim_alloc_coherent(struct device *dev, size_t size,
55047769b3e974 Jason Wang 2020-01-16 264
dma_addr_t *dma_addr, gfp_t flag,
55047769b3e974 Jason Wang 2020-01-16 265
unsigned long attrs)
55047769b3e974 Jason Wang 2020-01-16 266 {
55047769b3e974 Jason Wang 2020-01-16 267 struct vdpa_device *vdpa =
dev_to_vdpa(dev);
55047769b3e974 Jason Wang 2020-01-16 268 struct vdpasim *vdpasim =
vdpa_to_sim(vdpa);
55047769b3e974 Jason Wang 2020-01-16 269 struct vhost_iotlb *iommu =
vdpasim->iommu;
55047769b3e974 Jason Wang 2020-01-16 270 void *addr = kmalloc(size,
flag);
55047769b3e974 Jason Wang 2020-01-16 271 int ret;
55047769b3e974 Jason Wang 2020-01-16 272
55047769b3e974 Jason Wang 2020-01-16 273 if (!addr)
55047769b3e974 Jason Wang 2020-01-16 274 *dma_addr =
DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 275 else {
55047769b3e974 Jason Wang 2020-01-16 276 u64 pa =
virt_to_phys(addr);
55047769b3e974 Jason Wang 2020-01-16 277
55047769b3e974 Jason Wang 2020-01-16 278 ret =
vhost_iotlb_add_range(iommu, (u64)pa,
55047769b3e974 Jason Wang 2020-01-16 279
(u64)pa + size - 1,
55047769b3e974 Jason Wang 2020-01-16 280
pa, VHOST_MAP_RW);
55047769b3e974 Jason Wang 2020-01-16 281 if (ret) {
55047769b3e974 Jason Wang 2020-01-16 282 kfree(addr);
^^^
55047769b3e974 Jason Wang 2020-01-16 283 *dma_addr =
DMA_MAPPING_ERROR;
55047769b3e974 Jason Wang 2020-01-16 284 } else
55047769b3e974 Jason Wang 2020-01-16 285 *dma_addr =
(dma_addr_t)pa;
55047769b3e974 Jason Wang 2020-01-16 286 }
55047769b3e974 Jason Wang 2020-01-16 287
55047769b3e974 Jason Wang 2020-01-16 @288 return addr;
55047769b3e974 Jason Wang 2020-01-16 289 }
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org Intel Corporation
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] x86/paravirt: Use a single ops structure
Hello Juergen Gross,
The patch 5c83511bdb98: "x86/paravirt: Use a single ops structure"
from Aug 28, 2018, leads to the following static checker warning:
arch/x86/kernel/paravirt.c:123 paravirt_patch_default()
warn: uncapped user index '*(_ops + type)'
arch/x86/kernel/paravirt.c:124 paravirt_patch_default()
error: buffer overflow '_ops' 90 <= 255 user_rl='0-255'
arch/x86/kernel/paravirt.c
116 unsigned paravirt_patch_default(u8 type, void *insn_buff,
117 unsigned long addr, unsigned len)
118 {
119 /*
120 * Neat trick to map patch type back to the call within the
121 * corresponding structure.
122 */
123 void *opfunc = *((void **)_ops + type);
^^
This code is actually pretty old...
This isn't a security issue, but the size of _ops is variable but
type isn't checked so we could be reading beyond the end. We could do
something like:
if (type >= sizeof(pv_ops) / sizeof(void *))
return -EINVAL;
opfunc = *((void **)_ops + type);
124 unsigned ret;
125
126 if (opfunc == NULL)
127 /* If there's no function, patch it with a ud2a (BUG) */
128 ret = paravirt_patch_insns(insn_buff, len, ud2a,
ud2a+sizeof(ud2a));
129 else if (opfunc == _paravirt_nop)
130 ret = 0;
131
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR
On Wed, Jan 09, 2019 at 09:36:41PM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 09, 2019 at 11:35:52AM +0100, Miguel Ojeda wrote:
> > On Tue, Jan 8, 2019 at 6:44 PM Nick Desaulniers
> > wrote:
> > >
> > > Also for more context, see:
> > > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against
> > > dead store elimination")
> >
> > By the way, shouldn't that barrier_data() be directly in compiler.h
> > too, since it is for both gcc & clang?
> >
> > > Reviewed-by: Nick Desaulniers
> > >
> > > + Miguel
> > > Miguel, would you mind taking this into your compiler-attributes tree?
> >
> > Sure, at least we get quickly some linux-next time.
>
>
> BTW why linux-next? shouldn't this go into 5.0 and stable? It's a bugfix
> after all.
>
It doesn't hurt to put things in linux-next for a week and then 5.0 and
-stable. Not a lot of testing happens on linux-next, but some does.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH net-next] vhost_net: add a missing error return
We accidentally left out this error return so it leads to some use after
free bugs later on.
Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer
sockets")
Signed-off-by: Dan Carpenter
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index dd4e0a301635..1bff6bc8161a 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1244,6 +1244,7 @@ static int vhost_net_open(struct inode *inode, struct
file *f)
kfree(vqs);
kvfree(n);
kfree(queue);
+ return -ENOMEM;
}
n->vqs[VHOST_NET_VQ_TX].xdp = xdp;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] x86/paravirt: fix some warning messages
The first argument to WARN_ONCE() is a condition.
Fixes: 5800dc5c19f3 ("x86/paravirt: Fix spectre-v2 mitigations for paravirt
guests")
Signed-off-by: Dan Carpenter
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index bbf006fe78d7..e4d4df37922a 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -89,7 +89,7 @@ static unsigned paravirt_patch_call(void *insnbuf, const void
*target,
if (len < 5) {
#ifdef CONFIG_RETPOLINE
- WARN_ONCE("Failing to patch indirect CALL in %ps\n", (void
*)addr);
+ WARN_ONCE(1, "Failing to patch indirect CALL in %ps\n", (void
*)addr);
#endif
return len; /* call too long for patch site */
}
@@ -110,7 +110,7 @@ static unsigned paravirt_patch_jmp(void *insnbuf, const
void *target,
if (len < 5) {
#ifdef CONFIG_RETPOLINE
- WARN_ONCE("Failing to patch indirect JMP in %ps\n", (void
*)addr);
+ WARN_ONCE(1, "Failing to patch indirect JMP in %ps\n", (void
*)addr);
#endif
return len; /* call too long for patch site */
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: fix bounds check in virtio_gpu_cmd_get_capset()
This doesn't affect runtime because in the current code "idx" is always
valid.
First, we read from "vgdev->capsets[idx].max_size" before checking
whether "idx" is within bounds. And secondly the bounds check is off by
one so we could end up reading one element beyond the end of the
vgdev->capsets[] array.
Fixes: 62fb7a5e1096 ("virtio-gpu: add 3d/virgl support")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 020070d483d3..4735bd1c7321 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -648,11 +648,11 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device
*vgdev,
{
struct virtio_gpu_get_capset *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
- int max_size = vgdev->capsets[idx].max_size;
+ int max_size;
struct virtio_gpu_drv_cap_cache *cache_ent;
void *resp_buf;
- if (idx > vgdev->num_capsets)
+ if (idx >= vgdev->num_capsets)
return -EINVAL;
if (version > vgdev->capsets[idx].max_version)
@@ -662,6 +662,7 @@ int virtio_gpu_cmd_get_capset(struct virtio_gpu_device
*vgdev,
if (!cache_ent)
return -ENOMEM;
+ max_size = vgdev->capsets[idx].max_size;
cache_ent->caps_cache = kmalloc(max_size, GFP_KERNEL);
if (!cache_ent->caps_cache) {
kfree(cache_ent);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next v8 2/4] net: Introduce generic failover module
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180427-183842
smatch warnings:
net/core/net_failover.c:229 net_failover_change_mtu() error: we previously
assumed 'primary_dev' could be null (see line 219)
net/core/net_failover.c:279 net_failover_vlan_rx_add_vid() error: we previously
assumed 'primary_dev' could be null (see line 269)
#
https://github.com/0day-ci/linux/commit/5a5f2e3efcb699867db79543dfebe764927b9c93
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5a5f2e3efcb699867db79543dfebe764927b9c93
vim +/primary_dev +229 net/core/net_failover.c
5a5f2e3e Sridhar Samudrala 2018-04-25 211
5a5f2e3e Sridhar Samudrala 2018-04-25 212 static int
net_failover_change_mtu(struct net_device *dev, int new_mtu)
5a5f2e3e Sridhar Samudrala 2018-04-25 213 {
5a5f2e3e Sridhar Samudrala 2018-04-25 214 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 215 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 216 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 217
5a5f2e3e Sridhar Samudrala 2018-04-25 218 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 @219 if (primary_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 220 ret =
dev_set_mtu(primary_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 221 if (ret)
5a5f2e3e Sridhar Samudrala 2018-04-25 222 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 223 }
5a5f2e3e Sridhar Samudrala 2018-04-25 224
5a5f2e3e Sridhar Samudrala 2018-04-25 225 standby_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 226 if (standby_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 227 ret =
dev_set_mtu(standby_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 228 if (ret) {
5a5f2e3e Sridhar Samudrala 2018-04-25 @229
dev_set_mtu(primary_dev, dev->mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 230 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 231 }
5a5f2e3e Sridhar Samudrala 2018-04-25 232 }
5a5f2e3e Sridhar Samudrala 2018-04-25 233
5a5f2e3e Sridhar Samudrala 2018-04-25 234 dev->mtu = new_mtu;
5a5f2e3e Sridhar Samudrala 2018-04-25 235
5a5f2e3e Sridhar Samudrala 2018-04-25 236 return 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 237 }
5a5f2e3e Sridhar Samudrala 2018-04-25 238
5a5f2e3e Sridhar Samudrala 2018-04-25 239 static void
net_failover_set_rx_mode(struct net_device *dev)
5a5f2e3e Sridhar Samudrala 2018-04-25 240 {
5a5f2e3e Sridhar Samudrala 2018-04-25 241 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 242 struct net_device *slave_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 243
5a5f2e3e Sridhar Samudrala 2018-04-25 244 rcu_read_lock();
5a5f2e3e Sridhar Samudrala 2018-04-25 245
5a5f2e3e Sridhar Samudrala 2018-04-25 246 slave_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 247 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 248
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 249
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 250 }
5a5f2e3e Sridhar Samudrala 2018-04-25 251
5a5f2e3e Sridhar Samudrala 2018-04-25 252 slave_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 253 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 254
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 255
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 256 }
5a5f2e3e Sridhar Samudrala 2018-04-25 257
5a5f2e3e Sridhar Samudrala 2018-04-25 258 rcu_read_unlock();
5a5f2e3e Sridhar Samudrala 2018-04-25 259 }
5a5f2e3e Sridhar Samudrala 2018-04-25 260
5a5f2e3e Sridhar Samudrala 2018-04-25 261 static int
net_failover_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
5a5f2e3e Sridhar Samudrala 2018-04-25 262
u16 vid)
5a5f2e3e Sridhar Samudrala 2018-04-25 263 {
5a5f2e3e Sridhar Samudrala 2018-04-25 264 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 265 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 266 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 267
5a5f2e3e Sridhar Samudrala 2018-04-25 268 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar
Re: [PATCH] drm: qxl: ratelimit pr_info message, reduce log spamming
On Tue, Sep 12, 2017 at 03:02:04PM +0100, Emil Velikov wrote: > That said, I'm not sure how useful the information is - perhaps it's > better to drop it all together? Or a WARN_ONCE(). regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
On Mon, May 22, 2017 at 12:23:20PM +0100, Stefan Hajnoczi wrote: > I'm not sure if kmalloc() and friends guarantee to show > a message (not just the first time, but for every failed allocation)? > It prints multiple times, but it's ratelimited. It can also be disabled using a config option. See slab_out_of_memory(). regards, dan carpenter ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] ringtest: fix an assert statement
There is an || vs && typo so the assert can never be triggered. Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com> diff --git a/tools/virtio/ringtest/main.c b/tools/virtio/ringtest/main.c index 022ae95a06bd..453ca3c21193 100644 --- a/tools/virtio/ringtest/main.c +++ b/tools/virtio/ringtest/main.c @@ -87,7 +87,7 @@ void set_affinity(const char *arg) cpu = strtol(arg, , 0); assert(!*endptr); - assert(cpu >= 0 || cpu < CPU_SETSIZE); + assert(cpu >= 0 && cpu < CPU_SETSIZE); self = pthread_self(); CPU_ZERO(); ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio_net: tidy a couple debug statements
We are printing a decimal value for truesize so we shouldn't use an "0x"
prefix.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0e6e3acf6ff1..2b5e8069d9e5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -668,7 +668,7 @@ static struct sk_buff *receive_mergeable(struct net_device
*dev,
rcu_read_unlock();
if (unlikely(len > (unsigned long)ctx)) {
- pr_debug("%s: rx error: len %u exceeds truesize 0x%lu\n",
+ pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
dev->name, len, (unsigned long)ctx);
dev->stats.rx_length_errors++;
goto err_skb;
@@ -694,7 +694,7 @@ static struct sk_buff *receive_mergeable(struct net_device
*dev,
page = virt_to_head_page(buf);
if (unlikely(len > (unsigned long)ctx)) {
- pr_debug("%s: rx error: len %u exceeds truesize
0x%lu\n",
+ pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
dev->name, len, (unsigned long)ctx);
dev->stats.rx_length_errors++;
goto err_skb;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[bug report] virtio_net: rework mergeable buffer handling
Hello Michael S. Tsirkin,
The patch 6c8e5f3c41c8: "virtio_net: rework mergeable buffer
handling" from Mar 6, 2017, leads to the following static checker
warning:
drivers/net/virtio_net.c:1042 virtnet_receive()
error: uninitialized symbol 'ctx'.
drivers/net/virtio_net.c
1030 static int virtnet_receive(struct receive_queue *rq, int budget)
1031 {
1032 struct virtnet_info *vi = rq->vq->vdev->priv;
1033 unsigned int len, received = 0, bytes = 0;
1034 void *buf;
1035 struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
1036
1037 if (vi->mergeable_rx_bufs) {
1038 void *ctx;
^^^
1039
1040 while (received < budget &&
1041 (buf = virtqueue_get_buf_ctx(rq->vq, ,
))) {
1042 bytes += receive_buf(vi, rq, buf, len, ctx);
^^^
It's possible that this code is correct, but I looked at it and wasn't
immediately convinced. Returning non-NULL buf is not sufficient to
show that "ctx" is initialized, because if it's vq->indirect then "buf"
is still unintialized. Also it's possible that receive_buf() checks
vq->indirect through some side effect way that I didn't see so it
doesn't use the uninitialized value...
I feel like if this is a false positive, that means the rules are too
subtle... :/
1043 received++;
1044 }
1045 } else {
1046 while (received < budget &&
1047 (buf = virtqueue_get_buf(rq->vq, )) != NULL)
{
1048 bytes += receive_buf(vi, rq, buf, len, NULL);
1049 received++;
1050 }
1051 }
1052
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
