[PATCH rdma-rc] IB/core: Fix potential NULL pointer dereference in pkey cache

[PATCH rdma-rc] IB/core: Fix potential NULL pointer dereference in pkey cache

[Leon Romanovsky]

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The IB core pkey cache is populated by procedure ib_cache_update().
Initially, the pkey cache pointer is NULL. ib_cache_update allocates
a buffer and populates it with the device's pkeys, via repeated calls
to procedure ib_query_pkey().

If there is a failure in populating the pkey buffer via ib_query_pkey(),
ib_cache_update does not replace the old pkey buffer cache with the
updated one -- it leaves the old cache as is.

Since initially the pkey buffer cache is NULL, when calling
ib_cache_update the first time, a failure in ib_query_pkey() will cause
the pkey buffer cache pointer to remain NULL.

In this situation, any calls subsequent to ib_get_cached_pkey(),
ib_find_cached_pkey(), or ib_find_cached_pkey_exact() will try to
dereference the NULL pkey cache pointer, causing a kernel panic.

Fix this by testing if the cache pointer is NULL. If yes,
return -ENOENT.

Fixes: 8faea9fd4a39 ("RDMA/cache: Move the cache per-port data into the main ib_port_data")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/cache.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 717b798cddad..4263a482ecab 100644
--- a/drivers/infiniband/core/cache.c
+++ b/drivers/infiniband/core/cache.c
@@ -1054,11 +1054,17 @@ int ib_get_cached_pkey(struct ib_device *device,
 
 	cache = device->port_data[port_num].cache.pkey;
 
+	if (!cache) {
+		ret = -ENOENT;
+		goto out;
+	}
+
 	if (index < 0 || index >= cache->table_len)
 		ret = -EINVAL;
 	else
 		*pkey = cache->table[index];
 
+out:
 	read_unlock_irqrestore(&device->cache_lock, flags);
 
 	return ret;
@@ -1101,6 +1107,8 @@ int ib_find_cached_pkey(struct ib_device *device,
 	cache = device->port_data[port_num].cache.pkey;
 
 	*index = -1;
+	if (!cache)
+		goto out;
 
 	for (i = 0; i < cache->table_len; ++i)
 		if ((cache->table[i] & 0x7fff) == (pkey & 0x7fff)) {
@@ -1117,6 +1125,7 @@ int ib_find_cached_pkey(struct ib_device *device,
 		ret = 0;
 	}
 
+out:
 	read_unlock_irqrestore(&device->cache_lock, flags);
 
 	return ret;
@@ -1141,6 +1150,8 @@ int ib_find_exact_cached_pkey(struct ib_device *device,
 	cache = device->port_data[port_num].cache.pkey;
 
 	*index = -1;
+	if (!cache)
+		goto out;
 
 	for (i = 0; i < cache->table_len; ++i)
 		if (cache->table[i] == pkey) {
@@ -1149,6 +1160,7 @@ int ib_find_exact_cached_pkey(struct ib_device *device,
 			break;
 		}
 
+out:
 	read_unlock_irqrestore(&device->cache_lock, flags);
 
 	return ret;
-- 
2.25.3

Re: [PATCH rdma-rc] IB/core: Fix potential NULL pointer dereference in pkey cache

[Jason Gunthorpe]

On Sun, Apr 26, 2020 at 10:58:11AM +0300, Leon Romanovsky wrote:
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> 
> The IB core pkey cache is populated by procedure ib_cache_update().
> Initially, the pkey cache pointer is NULL. ib_cache_update allocates
> a buffer and populates it with the device's pkeys, via repeated calls
> to procedure ib_query_pkey().
> 
> If there is a failure in populating the pkey buffer via ib_query_pkey(),
> ib_cache_update does not replace the old pkey buffer cache with the
> updated one -- it leaves the old cache as is.

The bug described here is that ib_cache_setup_one() ignores the return
codes from ib_cache_update()

Device registration should fail if the cache could not be loaded, just
fix ib_cache_setup_one()

Jason

Re: [PATCH rdma-rc] IB/core: Fix potential NULL pointer dereference in pkey cache

[Leon Romanovsky]

On Mon, May 04, 2020 at 02:58:29PM -0300, Jason Gunthorpe wrote:
> On Sun, Apr 26, 2020 at 10:58:11AM +0300, Leon Romanovsky wrote:
> > From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> >
> > The IB core pkey cache is populated by procedure ib_cache_update().
> > Initially, the pkey cache pointer is NULL. ib_cache_update allocates
> > a buffer and populates it with the device's pkeys, via repeated calls
> > to procedure ib_query_pkey().
> >
> > If there is a failure in populating the pkey buffer via ib_query_pkey(),
> > ib_cache_update does not replace the old pkey buffer cache with the
> > updated one -- it leaves the old cache as is.
>
> The bug described here is that ib_cache_setup_one() ignores the return
> codes from ib_cache_update()
>
> Device registration should fail if the cache could not be loaded, just
> fix ib_cache_setup_one()

Thanks, it is much easy fix.

>
> Jason