All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix build error in function vfio_combine_iova_ranges
@ 2023-09-11  9:41 Cong Liu
  2023-09-11 18:44 ` Alex Williamson
  0 siblings, 1 reply; 11+ messages in thread
From: Cong Liu @ 2023-09-11  9:41 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Cong Liu, kvm, linux-kernel

when compiling with smatch check, the following errors were encountered:

drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.

this patch fix these error.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 drivers/vfio/vfio_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..0a9620409696 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -938,12 +938,13 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
 void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			      u32 req_nodes)
 {
-	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
+	struct interval_tree_node *prev, *curr;
+	struct interval_tree_node *comb_start, *comb_end;
 	unsigned long min_gap, curr_gap;
 
 	/* Special shortcut when a single range is required */
 	if (req_nodes == 1) {
-		unsigned long last;
+		unsigned long last = 0;
 
 		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
 		curr = comb_start;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix build error in function vfio_combine_iova_ranges
  2023-09-11  9:41 [PATCH] fix build error in function vfio_combine_iova_ranges Cong Liu
@ 2023-09-11 18:44 ` Alex Williamson
  2023-09-12  1:07   ` Cong Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Williamson @ 2023-09-11 18:44 UTC (permalink / raw)
  To: Cong Liu; +Cc: kvm, linux-kernel

On Mon, 11 Sep 2023 17:41:02 +0800
Cong Liu <liucong2@kylinos.cn> wrote:

> when compiling with smatch check, the following errors were encountered:
> 
> drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> this patch fix these error.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  drivers/vfio/vfio_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 40732e8ed4c6..0a9620409696 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -938,12 +938,13 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
>  void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  			      u32 req_nodes)
>  {
> -	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
> +	struct interval_tree_node *prev, *curr;
> +	struct interval_tree_node *comb_start, *comb_end;

This looks cosmetic, what did it fix?  It's not clear to me how any of
this addresses the last two errors.  Thanks,

Alex

>  	unsigned long min_gap, curr_gap;
>  
>  	/* Special shortcut when a single range is required */
>  	if (req_nodes == 1) {
> -		unsigned long last;
> +		unsigned long last = 0;
>  
>  		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
>  		curr = comb_start;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] fix build error in function vfio_combine_iova_ranges
  2023-09-11 18:44 ` Alex Williamson
@ 2023-09-12  1:07   ` Cong Liu
  2023-09-13 12:36     ` Jason Gunthorpe
  0 siblings, 1 reply; 11+ messages in thread
From: Cong Liu @ 2023-09-12  1:07 UTC (permalink / raw)
  To: alex.williamson; +Cc: kvm, linux-kernel, liucong2

when compiling with smatch check, the following errors were encountered:

drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.

this patch fix these error.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 drivers/vfio/vfio_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..68a0a5081161 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -938,12 +938,13 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
 void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			      u32 req_nodes)
 {
-	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
+	struct interval_tree_node *prev, *curr;
+	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;
 	unsigned long min_gap, curr_gap;
 
 	/* Special shortcut when a single range is required */
 	if (req_nodes == 1) {
-		unsigned long last;
+		unsigned long last = 0;
 
 		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
 		curr = comb_start;
-- 
2.34.1

I'm very sorry, I compiled the code on another machine and then copied the wrong patch.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] fix build error in function vfio_combine_iova_ranges
  2023-09-12  1:07   ` Cong Liu
@ 2023-09-13 12:36     ` Jason Gunthorpe
  2023-09-14  9:08       ` [PATCH v2] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges Cong Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Gunthorpe @ 2023-09-13 12:36 UTC (permalink / raw)
  To: Cong Liu; +Cc: alex.williamson, kvm, linux-kernel

On Tue, Sep 12, 2023 at 09:07:36AM +0800, Cong Liu wrote:
> when compiling with smatch check, the following errors were encountered:
> 
> drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> this patch fix these error.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  drivers/vfio/vfio_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 40732e8ed4c6..68a0a5081161 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -938,12 +938,13 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
>  void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  			      u32 req_nodes)
>  {
> -	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
> +	struct interval_tree_node *prev, *curr;
> +	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;
>  	unsigned long min_gap, curr_gap;
>  
>  	/* Special shortcut when a single range is required */
>  	if (req_nodes == 1) {
> -		unsigned long last;
> +		unsigned long last = 0;
>  
>  		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
>  		curr = comb_start;

These are not possible unless the list is empty, and assigning
zero/null isn't an improvement for that case it will just crash

Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges
  2023-09-13 12:36     ` Jason Gunthorpe
@ 2023-09-14  9:08       ` Cong Liu
  2023-09-19 18:04         ` Alex Williamson
  0 siblings, 1 reply; 11+ messages in thread
From: Cong Liu @ 2023-09-14  9:08 UTC (permalink / raw)
  To: jgg, Alex Williamson; +Cc: kvm, linux-kernel, liucong2

when compiling with smatch check, the following errors were encountered:

drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.

this patch fix these error.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 drivers/vfio/vfio_main.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..96d2f3030ebb 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -938,14 +938,17 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
 void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			      u32 req_nodes)
 {
-	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
+	struct interval_tree_node *prev, *curr;
+	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;
 	unsigned long min_gap, curr_gap;
 
 	/* Special shortcut when a single range is required */
 	if (req_nodes == 1) {
-		unsigned long last;
+		unsigned long last = 0;
 
 		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
+		if (!comb_start)
+			return;
 		curr = comb_start;
 		while (curr) {
 			last = curr->last;
@@ -963,6 +966,10 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 		prev = NULL;
 		min_gap = ULONG_MAX;
 		curr = interval_tree_iter_first(root, 0, ULONG_MAX);
+		if (!curr) {
+			/* No more ranges to combine */
+			break;
+		}
 		while (curr) {
 			if (prev) {
 				curr_gap = curr->start - prev->last;
@@ -975,6 +982,10 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			prev = curr;
 			curr = interval_tree_iter_next(curr, 0, ULONG_MAX);
 		}
+		if (!comb_start || !comb_end) {
+			/* No more ranges to combine */
+			break;
+		}
 		comb_start->last = comb_end->last;
 		interval_tree_remove(comb_end, root);
 		cur_nodes--;
-- 
2.34.1
While this may seem somewhat verbose, I am unsure how to refactor this function.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges
  2023-09-14  9:08       ` [PATCH v2] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges Cong Liu
@ 2023-09-19 18:04         ` Alex Williamson
  2023-09-20  9:55           ` [PATCH v3] " Cong Liu
  0 siblings, 1 reply; 11+ messages in thread
From: Alex Williamson @ 2023-09-19 18:04 UTC (permalink / raw)
  To: Cong Liu; +Cc: jgg, kvm, linux-kernel

On Thu, 14 Sep 2023 17:08:39 +0800
Cong Liu <liucong2@kylinos.cn> wrote:

> when compiling with smatch check, the following errors were encountered:
> 
> drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> this patch fix these error.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  drivers/vfio/vfio_main.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 40732e8ed4c6..96d2f3030ebb 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -938,14 +938,17 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
>  void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  			      u32 req_nodes)
>  {
> -	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
> +	struct interval_tree_node *prev, *curr;
> +	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;
>  	unsigned long min_gap, curr_gap;
>  
>  	/* Special shortcut when a single range is required */
>  	if (req_nodes == 1) {
> -		unsigned long last;
> +		unsigned long last = 0;
>  
>  		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
> +		if (!comb_start)
> +			return;
>  		curr = comb_start;
>  		while (curr) {
>  			last = curr->last;

@last no longer requires initialization with the @comb_start test.

However, all of these are testing for invalid parameters, which I think
we can eliminate if we simply introduce the following at the start of
the function:

        if (!cur_nodes || cur_nodes <= req_nodes ||
            WARN_ON(!req_nodes || !root->rb_root.rb_node))
                return;

At that point we're guaranteed to have any entry for both the above and
below first entry and there must be at least a second entry (or a
driver bug telling us there are more entries than actually exist) for
the next call below.  Thanks,

Alex


> @@ -963,6 +966,10 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  		prev = NULL;
>  		min_gap = ULONG_MAX;
>  		curr = interval_tree_iter_first(root, 0, ULONG_MAX);
> +		if (!curr) {
> +			/* No more ranges to combine */
> +			break;
> +		}
>  		while (curr) {
>  			if (prev) {
>  				curr_gap = curr->start - prev->last;
> @@ -975,6 +982,10 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  			prev = curr;
>  			curr = interval_tree_iter_next(curr, 0, ULONG_MAX);
>  		}
> +		if (!comb_start || !comb_end) {
> +			/* No more ranges to combine */
> +			break;
> +		}
>  		comb_start->last = comb_end->last;
>  		interval_tree_remove(comb_end, root);
>  		cur_nodes--;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges
  2023-09-19 18:04         ` Alex Williamson
@ 2023-09-20  9:55           ` Cong Liu
  2023-10-02 22:41             ` Alex Williamson
  2023-10-02 22:43             ` [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges() Alex Williamson
  0 siblings, 2 replies; 11+ messages in thread
From: Cong Liu @ 2023-09-20  9:55 UTC (permalink / raw)
  To: alex.williamson; +Cc: jgg, kvm, linux-kernel, liucong2

when compiling with smatch check, the following errors were encountered:

drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.

This patch initializes the variables last, comb_end, and comb_start
to avoid compiler warnings and add proper argument checks to ensure
interval_tree_iter_first() does not return NULL.

Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
 drivers/vfio/vfio_main.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..ecd4dd8e6b05 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -938,12 +938,17 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
 void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			      u32 req_nodes)
 {
-	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
+	if (!cur_nodes || cur_nodes <= req_nodes ||
+		WARN_ON(!req_nodes || !root->rb_root.rb_node))
+		return;
+
+	struct interval_tree_node *prev, *curr;
+	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;
 	unsigned long min_gap, curr_gap;
 
 	/* Special shortcut when a single range is required */
 	if (req_nodes == 1) {
-		unsigned long last;
+		unsigned long last = 0;
 
 		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
 		curr = comb_start;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges
  2023-09-20  9:55           ` [PATCH v3] " Cong Liu
@ 2023-10-02 22:41             ` Alex Williamson
  2023-10-02 22:43             ` [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges() Alex Williamson
  1 sibling, 0 replies; 11+ messages in thread
From: Alex Williamson @ 2023-10-02 22:41 UTC (permalink / raw)
  To: Cong Liu; +Cc: jgg, kvm, linux-kernel

On Wed, 20 Sep 2023 17:55:31 +0800
Cong Liu <liucong2@kylinos.cn> wrote:

> when compiling with smatch check, the following errors were encountered:
> 
> drivers/vfio/vfio_main.c:957 vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> drivers/vfio/vfio_main.c:978 vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> This patch initializes the variables last, comb_end, and comb_start
> to avoid compiler warnings and add proper argument checks to ensure
> interval_tree_iter_first() does not return NULL.
> 
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
>  drivers/vfio/vfio_main.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 40732e8ed4c6..ecd4dd8e6b05 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -938,12 +938,17 @@ static int vfio_ioctl_device_feature_migration(struct vfio_device *device,
>  void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>  			      u32 req_nodes)
>  {
> -	struct interval_tree_node *prev, *curr, *comb_start, *comb_end;
> +	if (!cur_nodes || cur_nodes <= req_nodes ||
> +		WARN_ON(!req_nodes || !root->rb_root.rb_node))
> +		return;

Code should not precede variable declaration.  The wrapped line should
align inside the parenthesis of the previous line.

> +
> +	struct interval_tree_node *prev, *curr;
> +	struct interval_tree_node *comb_start = NULL, *comb_end = NULL;

These only mask the issue reported by smatch, should we encounter the
condition where these are used uninitialized, derefencing NULL is only
marginally better.

>  	unsigned long min_gap, curr_gap;
>  
>  	/* Special shortcut when a single range is required */
>  	if (req_nodes == 1) {
> -		unsigned long last;
> +		unsigned long last = 0;

This also masks the actual error, which can only occur from an empty
list which still results in a NULL pointer derefence.  Should we even
make use of @last, a zero value is arbitrary.

I'll follow-up with a patch that I believe better resolves the report.
Thanks,

Alex

>  
>  		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
>  		curr = comb_start;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges()
  2023-09-20  9:55           ` [PATCH v3] " Cong Liu
  2023-10-02 22:41             ` Alex Williamson
@ 2023-10-02 22:43             ` Alex Williamson
  2023-10-04 15:35               ` Brett Creeley
  2023-10-06 16:56               ` Jason Gunthorpe
  1 sibling, 2 replies; 11+ messages in thread
From: Alex Williamson @ 2023-10-02 22:43 UTC (permalink / raw)
  To: alex.williamson; +Cc: kvm, linux-kernel, liucong2, yishaih, brett.creeley

smatch reports:

vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.

These errors are only reachable via invalid input, in the case of
@last when we receive an empty rb-tree or for @comb_{start,end} if the
rb-tree is empty or otherwise fails to produce a second node that
reduces the gap.  Add tests with warnings for these cases.

Reported-by: Cong Liu <liucong2@kylinos.cn>
Link: https://lore.kernel.org/all/20230920095532.88135-1-liucong2@kylinos.cn
Cc: Yishai Hadas <yishaih@nvidia.com>
Cc: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 40732e8ed4c6..e31e1952d7b8 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -946,6 +946,11 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 		unsigned long last;
 
 		comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
+
+		/* Empty list */
+		if (WARN_ON_ONCE(!comb_start))
+			return;
+
 		curr = comb_start;
 		while (curr) {
 			last = curr->last;
@@ -975,6 +980,11 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
 			prev = curr;
 			curr = interval_tree_iter_next(curr, 0, ULONG_MAX);
 		}
+
+		/* Empty list or no nodes to combine */
+		if (WARN_ON_ONCE(min_gap == ULONG_MAX))
+			break;
+
 		comb_start->last = comb_end->last;
 		interval_tree_remove(comb_end, root);
 		cur_nodes--;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges()
  2023-10-02 22:43             ` [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges() Alex Williamson
@ 2023-10-04 15:35               ` Brett Creeley
  2023-10-06 16:56               ` Jason Gunthorpe
  1 sibling, 0 replies; 11+ messages in thread
From: Brett Creeley @ 2023-10-04 15:35 UTC (permalink / raw)
  To: Alex Williamson; +Cc: kvm, linux-kernel, liucong2, yishaih, brett.creeley

On 10/2/2023 3:43 PM, Alex Williamson wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> smatch reports:
> 
> vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> These errors are only reachable via invalid input, in the case of
> @last when we receive an empty rb-tree or for @comb_{start,end} if the
> rb-tree is empty or otherwise fails to produce a second node that
> reduces the gap.  Add tests with warnings for these cases.
> 
> Reported-by: Cong Liu <liucong2@kylinos.cn>
> Link: https://lore.kernel.org/all/20230920095532.88135-1-liucong2@kylinos.cn
> Cc: Yishai Hadas <yishaih@nvidia.com>
> Cc: Brett Creeley <brett.creeley@amd.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>   drivers/vfio/vfio_main.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 40732e8ed4c6..e31e1952d7b8 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -946,6 +946,11 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>                  unsigned long last;
> 
>                  comb_start = interval_tree_iter_first(root, 0, ULONG_MAX);
> +
> +               /* Empty list */
> +               if (WARN_ON_ONCE(!comb_start))
> +                       return;
> +
>                  curr = comb_start;
>                  while (curr) {
>                          last = curr->last;
> @@ -975,6 +980,11 @@ void vfio_combine_iova_ranges(struct rb_root_cached *root, u32 cur_nodes,
>                          prev = curr;
>                          curr = interval_tree_iter_next(curr, 0, ULONG_MAX);
>                  }
> +
> +               /* Empty list or no nodes to combine */
> +               if (WARN_ON_ONCE(min_gap == ULONG_MAX))
> +                       break;
> +
>                  comb_start->last = comb_end->last;
>                  interval_tree_remove(comb_end, root);
>                  cur_nodes--;
> --
> 2.40.1
> 

LGTM. Thanks for fixing this.

Reviewed-by: Brett Creeley <brett.creeley@amd.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges()
  2023-10-02 22:43             ` [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges() Alex Williamson
  2023-10-04 15:35               ` Brett Creeley
@ 2023-10-06 16:56               ` Jason Gunthorpe
  1 sibling, 0 replies; 11+ messages in thread
From: Jason Gunthorpe @ 2023-10-06 16:56 UTC (permalink / raw)
  To: Alex Williamson; +Cc: kvm, linux-kernel, liucong2, yishaih, brett.creeley

On Mon, Oct 02, 2023 at 04:43:25PM -0600, Alex Williamson wrote:
> smatch reports:
> 
> vfio_combine_iova_ranges() error: uninitialized symbol 'last'.
> vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_end'.
> vfio_combine_iova_ranges() error: potentially dereferencing uninitialized 'comb_start'.
> 
> These errors are only reachable via invalid input, in the case of
> @last when we receive an empty rb-tree or for @comb_{start,end} if the
> rb-tree is empty or otherwise fails to produce a second node that
> reduces the gap.  Add tests with warnings for these cases.
> 
> Reported-by: Cong Liu <liucong2@kylinos.cn>
> Link: https://lore.kernel.org/all/20230920095532.88135-1-liucong2@kylinos.cn
> Cc: Yishai Hadas <yishaih@nvidia.com>
> Cc: Brett Creeley <brett.creeley@amd.com>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio_main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Yeah, this is much clearer

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks,
Jason

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-10-06 16:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-11  9:41 [PATCH] fix build error in function vfio_combine_iova_ranges Cong Liu
2023-09-11 18:44 ` Alex Williamson
2023-09-12  1:07   ` Cong Liu
2023-09-13 12:36     ` Jason Gunthorpe
2023-09-14  9:08       ` [PATCH v2] vfio: Fix uninitialized symbol and potential dereferencing errors in vfio_combine_iova_ranges Cong Liu
2023-09-19 18:04         ` Alex Williamson
2023-09-20  9:55           ` [PATCH v3] " Cong Liu
2023-10-02 22:41             ` Alex Williamson
2023-10-02 22:43             ` [PATCH] vfio: Fix smatch errors in vfio_combine_iova_ranges() Alex Williamson
2023-10-04 15:35               ` Brett Creeley
2023-10-06 16:56               ` Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.