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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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