[PATCH] staging: vme_user: check find_bridge() return value

[PATCH] staging: vme_user: check find_bridge() return value

[Shyam Sunder Reddy Padira]

find_bridge() returns NULL when no matching bridge is found
for a given resource. Some call sites dereference the return
value without verifying it is non-NULL.

Add NULL checks before use to avoid potentail NULL pointer
dereferences.

Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira@gmail.com>
---
 drivers/staging/vme_user/vme.c | 63 ++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
index b5c66b66ce32..2ed2f1fe502f 100644
--- a/drivers/staging/vme_user/vme.c
+++ b/drivers/staging/vme_user/vme.c
@@ -82,6 +82,9 @@ void *vme_alloc_consistent(struct vme_resource *resource, size_t size,
 {
 	struct vme_bridge *bridge = find_bridge(resource);
 
+	if (!bridge)
+		return NULL;
+
 	if (!bridge->alloc_consistent) {
 		dev_err(bridge->parent,
 			"alloc_consistent not supported by bridge %s\n",
@@ -107,6 +110,9 @@ void vme_free_consistent(struct vme_resource *resource, size_t size,
 {
 	struct vme_bridge *bridge = find_bridge(resource);
 
+	if (!bridge)
+		return;
+
 	if (!bridge->free_consistent) {
 		dev_err(bridge->parent,
 			"free_consistent not supported by bridge %s\n",
@@ -136,6 +142,9 @@ size_t vme_get_size(struct vme_resource *resource)
 	dma_addr_t buf_base;
 	u32 aspace, cycle, dwidth;
 
+	if (!bridge)
+		return 0;
+
 	switch (resource->type) {
 	case VME_MASTER:
 		retval = vme_master_get(resource, &enabled, &base, &size,
@@ -332,6 +341,9 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
 	struct vme_slave_resource *image;
 	int retval;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_SLAVE) {
 		dev_err(bridge->parent, "Not a slave resource\n");
 		return -EINVAL;
@@ -381,6 +393,9 @@ int vme_slave_get(struct vme_resource *resource, int *enabled,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_slave_resource *image;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_SLAVE) {
 		dev_err(bridge->parent, "Not a slave resource\n");
 		return -EINVAL;
@@ -409,6 +424,9 @@ void vme_slave_free(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_slave_resource *slave_image;
 
+	if (!bridge)
+		return;
+
 	if (resource->type != VME_SLAVE) {
 		dev_err(bridge->parent, "Not a slave resource\n");
 		return;
@@ -529,6 +547,9 @@ int vme_master_set(struct vme_resource *resource, int enabled,
 	struct vme_master_resource *image;
 	int retval;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_MASTER) {
 		dev_err(bridge->parent, "Not a master resource\n");
 		return -EINVAL;
@@ -579,6 +600,9 @@ int vme_master_get(struct vme_resource *resource, int *enabled,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_master_resource *image;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_MASTER) {
 		dev_err(bridge->parent, "Not a master resource\n");
 		return -EINVAL;
@@ -618,6 +642,9 @@ ssize_t vme_master_read(struct vme_resource *resource, void *buf, size_t count,
 	struct vme_master_resource *image;
 	size_t length;
 
+	if (!bridge)
+		return 0;
+
 	if (!bridge->master_read) {
 		dev_warn(bridge->parent,
 			 "Reading from resource not supported\n");
@@ -667,6 +694,9 @@ ssize_t vme_master_write(struct vme_resource *resource, void *buf,
 	struct vme_master_resource *image;
 	size_t length;
 
+	if (!bridge)
+		return 0;
+
 	if (!bridge->master_write) {
 		dev_warn(bridge->parent, "Writing to resource not supported\n");
 		return -EINVAL;
@@ -718,6 +748,9 @@ unsigned int vme_master_rmw(struct vme_resource *resource, unsigned int mask,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_master_resource *image;
 
+	if (!bridge)
+		return 0;
+
 	if (!bridge->master_rmw) {
 		dev_warn(bridge->parent, "Writing to resource not supported\n");
 		return -EINVAL;
@@ -753,6 +786,9 @@ int vme_master_mmap_prepare(struct vme_resource *resource,
 	struct vme_master_resource *image;
 	phys_addr_t phys_addr;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_MASTER) {
 		dev_err(bridge->parent, "Not a master resource\n");
 		return -EINVAL;
@@ -783,6 +819,9 @@ void vme_master_free(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_master_resource *master_image;
 
+	if (!bridge)
+		return;
+
 	if (resource->type != VME_MASTER) {
 		dev_err(bridge->parent, "Not a master resource\n");
 		return;
@@ -889,6 +928,9 @@ struct vme_dma_list *vme_new_dma_list(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_dma_list *dma_list;
 
+	if (!bridge)
+		return NULL;
+
 	if (resource->type != VME_DMA) {
 		dev_err(bridge->parent, "Not a DMA resource\n");
 		return NULL;
@@ -1173,6 +1215,9 @@ int vme_dma_free(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_dma_resource *ctrlr;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_DMA) {
 		dev_err(bridge->parent, "Not a DMA resource\n");
 		return -EINVAL;
@@ -1494,6 +1539,9 @@ int vme_lm_count(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return -EINVAL;
@@ -1525,6 +1573,9 @@ int vme_lm_set(struct vme_resource *resource, unsigned long long lm_base,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return -EINVAL;
@@ -1561,6 +1612,9 @@ int vme_lm_get(struct vme_resource *resource, unsigned long long *lm_base,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return -EINVAL;
@@ -1598,6 +1652,9 @@ int vme_lm_attach(struct vme_resource *resource, int monitor,
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return -EINVAL;
@@ -1631,6 +1688,9 @@ int vme_lm_detach(struct vme_resource *resource, int monitor)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return 0;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return -EINVAL;
@@ -1664,6 +1724,9 @@ void vme_lm_free(struct vme_resource *resource)
 	struct vme_bridge *bridge = find_bridge(resource);
 	struct vme_lm_resource *lm;
 
+	if (!bridge)
+		return;
+
 	if (resource->type != VME_LM) {
 		dev_err(bridge->parent, "Not a Location Monitor resource\n");
 		return;
-- 
2.43.0

Re: [PATCH] staging: vme_user: check find_bridge() return value

[Greg KH]

On Wed, May 06, 2026 at 02:27:46AM +0530, Shyam Sunder Reddy Padira wrote:
> find_bridge() returns NULL when no matching bridge is found
> for a given resource. Some call sites dereference the return
> value without verifying it is non-NULL.
> 
> Add NULL checks before use to avoid potentail NULL pointer
> dereferences.
> 
> Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira@gmail.com>
> ---
>  drivers/staging/vme_user/vme.c | 63 ++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> index b5c66b66ce32..2ed2f1fe502f 100644
> --- a/drivers/staging/vme_user/vme.c
> +++ b/drivers/staging/vme_user/vme.c
> @@ -82,6 +82,9 @@ void *vme_alloc_consistent(struct vme_resource *resource, size_t size,
>  {
>  	struct vme_bridge *bridge = find_bridge(resource);
>  
> +	if (!bridge)
> +		return NULL;
> +
>  	if (!bridge->alloc_consistent) {
>  		dev_err(bridge->parent,
>  			"alloc_consistent not supported by bridge %s\n",
> @@ -107,6 +110,9 @@ void vme_free_consistent(struct vme_resource *resource, size_t size,
>  {
>  	struct vme_bridge *bridge = find_bridge(resource);
>  
> +	if (!bridge)
> +		return;
> +
>  	if (!bridge->free_consistent) {
>  		dev_err(bridge->parent,
>  			"free_consistent not supported by bridge %s\n",
> @@ -136,6 +142,9 @@ size_t vme_get_size(struct vme_resource *resource)
>  	dma_addr_t buf_base;
>  	u32 aspace, cycle, dwidth;
>  
> +	if (!bridge)
> +		return 0;

This is an error, why not return a error?

> +
>  	switch (resource->type) {
>  	case VME_MASTER:
>  		retval = vme_master_get(resource, &enabled, &base, &size,
> @@ -332,6 +341,9 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
>  	struct vme_slave_resource *image;
>  	int retval;
>  
> +	if (!bridge)
> +		return 0;

Same here.

Also, can this ever actually happen?  Given that the code seems to work
just fine, if you look at the callers, the function does not seem to
ever be able to return NULL, so be careful of adding checks that are
never actually needed.

thanks,

greg k-h

Re: [PATCH] staging: vme_user: check find_bridge() return value

[Shyam Sunder Reddy Padira]

Hi Greg,

Thanks for the review.

On re-checking the call paths, the resource passed to find_bridge()
originates from vme_user_probe(), where it is allocated via
vme_master_request(), vme_slave_request(), etc. These APIs set
resource->type to a valid VME_* value at creation time.

Since find_bridge() only returns NULL in the default case, and
resource->type is always one of the valid types, this path does not
appear to be reachable in practice.
Given this, the NULL check I added is unnecessary. I will drop this change.

Thanks,
Shyam


On Mon, 11 May 2026 at 13:16, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, May 06, 2026 at 02:27:46AM +0530, Shyam Sunder Reddy Padira wrote:
> > find_bridge() returns NULL when no matching bridge is found
> > for a given resource. Some call sites dereference the return
> > value without verifying it is non-NULL.
> >
> > Add NULL checks before use to avoid potentail NULL pointer
> > dereferences.
> >
> > Signed-off-by: Shyam Sunder Reddy Padira <shyamsunderreddypadira@gmail.com>
> > ---
> >  drivers/staging/vme_user/vme.c | 63 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >
> > diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> > index b5c66b66ce32..2ed2f1fe502f 100644
> > --- a/drivers/staging/vme_user/vme.c
> > +++ b/drivers/staging/vme_user/vme.c
> > @@ -82,6 +82,9 @@ void *vme_alloc_consistent(struct vme_resource *resource, size_t size,
> >  {
> >       struct vme_bridge *bridge = find_bridge(resource);
> >
> > +     if (!bridge)
> > +             return NULL;
> > +
> >       if (!bridge->alloc_consistent) {
> >               dev_err(bridge->parent,
> >                       "alloc_consistent not supported by bridge %s\n",
> > @@ -107,6 +110,9 @@ void vme_free_consistent(struct vme_resource *resource, size_t size,
> >  {
> >       struct vme_bridge *bridge = find_bridge(resource);
> >
> > +     if (!bridge)
> > +             return;
> > +
> >       if (!bridge->free_consistent) {
> >               dev_err(bridge->parent,
> >                       "free_consistent not supported by bridge %s\n",
> > @@ -136,6 +142,9 @@ size_t vme_get_size(struct vme_resource *resource)
> >       dma_addr_t buf_base;
> >       u32 aspace, cycle, dwidth;
> >
> > +     if (!bridge)
> > +             return 0;
>
> This is an error, why not return a error?
>
> > +
> >       switch (resource->type) {
> >       case VME_MASTER:
> >               retval = vme_master_get(resource, &enabled, &base, &size,
> > @@ -332,6 +341,9 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
> >       struct vme_slave_resource *image;
> >       int retval;
> >
> > +     if (!bridge)
> > +             return 0;
>
> Same here.
>
> Also, can this ever actually happen?  Given that the code seems to work
> just fine, if you look at the callers, the function does not seem to
> ever be able to return NULL, so be careful of adding checks that are
> never actually needed.
>
> thanks,
>
> greg k-h