* [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
@ 2020-08-18 14:25 Kamal Heib
2020-08-18 16:31 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Kamal Heib @ 2020-08-18 14:25 UTC (permalink / raw)
To: linux-rdma
Cc: Doug Ledford, Jason Gunthorpe, Leon Romanovsky, Zhu Yanjun, Kamal Heib
To avoid the following kernel panic when calling kmem_cache_create()
with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
context of device initialization.
BUG: unable to handle kernel NULL pointer dereference at 000000000000000b
PGD 0 P4D 0
Oops: 0000 [#1] SMP NOPTI
CPU: 4 PID: 8512 Comm: modprobe Kdump: loaded Not tainted 4.18.0-231.el8.x86_64 #1
Hardware name: HPE ProLiant DL385 Gen10/ProLiant DL385 Gen10, BIOS A40 10/02/2018
RIP: 0010:kmem_cache_alloc+0xd1/0x1b0
Code: 8b 57 18 45 8b 77 1c 48 8b 5c 24 30 0f 1f 44 00 00 5b 48 89 e8 5d 41 5c 41 5d 41 5e 41 5f c3 81 e3 00 00 10 00 75 0e 4d 89 fe <41> f6 47 0b 04 0f 84 6c ff ff ff 4c 89 ff e8 cc da 01 00 49 89 c6
RSP: 0018:ffffa2b8c773f9d0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000005
RDX: 0000000000000004 RSI: 00000000006080c0 RDI: 0000000000000000
RBP: ffff8ea0a8634fd0 R08: ffffa2b8c773f988 R09: 00000000006000c0
R10: 0000000000000000 R11: 0000000000000230 R12: 00000000006080c0
R13: ffffffffc0a97fc8 R14: 0000000000000000 R15: 0000000000000000
FS: 00007f9138ed9740(0000) GS:ffff8ea4ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000000b CR3: 000000046d59a000 CR4: 00000000003406e0
Call Trace:
rxe_alloc+0xc8/0x160 [rdma_rxe]
rxe_get_dma_mr+0x25/0xb0 [rdma_rxe]
__ib_alloc_pd+0xcb/0x160 [ib_core]
ib_mad_init_device+0x296/0x8b0 [ib_core]
add_client_context+0x11a/0x160 [ib_core]
enable_device_and_get+0xdc/0x1d0 [ib_core]
ib_register_device+0x572/0x6b0 [ib_core]
? crypto_create_tfm+0x32/0xe0
? crypto_create_tfm+0x7a/0xe0
? crypto_alloc_tfm+0x58/0xf0
rxe_register_device+0x19d/0x1c0 [rdma_rxe]
rxe_net_add+0x3d/0x70 [rdma_rxe]
? dev_get_by_name_rcu+0x73/0x90
rxe_param_set_add+0xaf/0xc0 [rdma_rxe]
parse_args+0x179/0x370
? ref_module+0x1b0/0x1b0
load_module+0x135e/0x17e0
? ref_module+0x1b0/0x1b0
? __do_sys_init_module+0x13b/0x180
__do_sys_init_module+0x13b/0x180
do_syscall_64+0x5b/0x1a0
entry_SYSCALL_64_after_hwframe+0x65/0xca
RIP: 0033:0x7f9137ed296e
Fixes: 8700e3e7c485 ("Soft RoCE driver")
Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
---
v2: Call rxe_cache_init() from rxe_init().
---
drivers/infiniband/sw/rxe/rxe.c | 14 +++++++-------
drivers/infiniband/sw/rxe/rxe_pool.c | 3 +++
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 5642eefb4ba1..c80f7c183957 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -252,6 +252,13 @@ static int rxe_init(struct rxe_dev *rxe)
/* init default device parameters */
rxe_init_device_param(rxe);
+ /* initialize slab caches for managed objects */
+ err = rxe_cache_init();
+ if (err) {
+ pr_err("unable to init object pools\n");
+ return err;
+ }
+
err = rxe_init_ports(rxe);
if (err)
goto err1;
@@ -336,13 +343,6 @@ static int __init rxe_module_init(void)
{
int err;
- /* initialize slab caches for managed objects */
- err = rxe_cache_init();
- if (err) {
- pr_err("unable to init object pools\n");
- return err;
- }
-
err = rxe_net_init();
if (err)
return err;
diff --git a/drivers/infiniband/sw/rxe/rxe_pool.c b/drivers/infiniband/sw/rxe/rxe_pool.c
index fbcbac52290b..06c6d1f835b7 100644
--- a/drivers/infiniband/sw/rxe/rxe_pool.c
+++ b/drivers/infiniband/sw/rxe/rxe_pool.c
@@ -139,6 +139,9 @@ int rxe_cache_init(void)
for (i = 0; i < RXE_NUM_TYPES; i++) {
type = &rxe_type_info[i];
size = ALIGN(type->size, RXE_POOL_ALIGN);
+ if (type->cache)
+ continue;
+
if (!(type->flags & RXE_POOL_NO_ALLOC)) {
type->cache =
kmem_cache_create(type->name, size,
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-18 14:25 [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create() Kamal Heib
@ 2020-08-18 16:31 ` Jason Gunthorpe
2020-08-18 21:15 ` Kamal Heib
0 siblings, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2020-08-18 16:31 UTC (permalink / raw)
To: Kamal Heib; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> To avoid the following kernel panic when calling kmem_cache_create()
> with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> context of device initialization.
I think you've hit on a bigger bug than just this oops.
rxe_net_add() should never be called before rxe_module_init(), that
surely subtly breaks all kinds of things.
Maybe it is time to remove these module parameters?
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-18 16:31 ` Jason Gunthorpe
@ 2020-08-18 21:15 ` Kamal Heib
2020-08-20 11:37 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Kamal Heib @ 2020-08-18 21:15 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > To avoid the following kernel panic when calling kmem_cache_create()
> > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > context of device initialization.
>
> I think you've hit on a bigger bug than just this oops.
>
> rxe_net_add() should never be called before rxe_module_init(), that
> surely subtly breaks all kinds of things.
>
> Maybe it is time to remove these module parameters?
>
Yes, I agree, this can be done in for-next.
But at least can we take this patch to for-rc (stable) to fix this issue
in stable releases?
Thanks,
Kamal
> Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-18 21:15 ` Kamal Heib
@ 2020-08-20 11:37 ` Jason Gunthorpe
2020-08-23 19:45 ` Kamal Heib
0 siblings, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2020-08-20 11:37 UTC (permalink / raw)
To: Kamal Heib; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Wed, Aug 19, 2020 at 12:15:45AM +0300, Kamal Heib wrote:
> On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> > On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > > To avoid the following kernel panic when calling kmem_cache_create()
> > > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > > context of device initialization.
> >
> > I think you've hit on a bigger bug than just this oops.
> >
> > rxe_net_add() should never be called before rxe_module_init(), that
> > surely subtly breaks all kinds of things.
> >
> > Maybe it is time to remove these module parameters?
> >
> Yes, I agree, this can be done in for-next.
>
> But at least can we take this patch to for-rc (stable) to fix this issue
> in stable releases?
If you want to fix something in stable then block the module options
from working as actual module options - eg before rxe_module_init()
runs.
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-20 11:37 ` Jason Gunthorpe
@ 2020-08-23 19:45 ` Kamal Heib
2020-08-24 6:23 ` Leon Romanovsky
2020-08-24 13:47 ` Jason Gunthorpe
0 siblings, 2 replies; 8+ messages in thread
From: Kamal Heib @ 2020-08-23 19:45 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Thu, Aug 20, 2020 at 08:37:17AM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 19, 2020 at 12:15:45AM +0300, Kamal Heib wrote:
> > On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> > > On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > > > To avoid the following kernel panic when calling kmem_cache_create()
> > > > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > > > context of device initialization.
> > >
> > > I think you've hit on a bigger bug than just this oops.
> > >
> > > rxe_net_add() should never be called before rxe_module_init(), that
> > > surely subtly breaks all kinds of things.
> > >
> > > Maybe it is time to remove these module parameters?
> > >
> > Yes, I agree, this can be done in for-next.
> >
> > But at least can we take this patch to for-rc (stable) to fix this issue
> > in stable releases?
>
> If you want to fix something in stable then block the module options
> from working as actual module options - eg before rxe_module_init()
> runs.
>
> Jason
Something like the following patch?
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 907203afbd99..872ebc57ac06 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -40,6 +40,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");
MODULE_DESCRIPTION("Soft RDMA transport");
MODULE_LICENSE("Dual BSD/GPL");
+bool rxe_is_loaded = false;
+
/* free resources for a rxe device all objects created for this device must
* have been destroyed
*/
@@ -315,6 +317,7 @@ static int __init rxe_module_init(void)
return err;
rdma_link_register(&rxe_link_ops);
+ rxe_is_loaded = true;
pr_info("loaded\n");
return 0;
}
@@ -326,6 +329,7 @@ static void __exit rxe_module_exit(void)
rxe_net_exit();
rxe_cache_exit();
+ rxe_is_loaded = false;
pr_info("unloaded\n");
}
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index fb07eed9e402..d9b71b5e2fba 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -67,6 +67,8 @@
#define RXE_ROCE_V2_SPORT (0xc000)
+extern bool rxe_is_loaded;
+
static inline u32 rxe_crc32(struct rxe_dev *rxe,
u32 crc, void *next, size_t len)
{
diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
index ccda5f5a3bc0..12c7ca0764d5 100644
--- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
+++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
@@ -61,6 +61,11 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
struct net_device *ndev;
struct rxe_dev *exists;
+ if (!rxe_is_loaded) {
+ pr_err("Please make sure to load the rdma_rxe module first\n");
+ return -EINVAL;
+ }
+
len = sanitize_arg(val, intf, sizeof(intf));
if (!len) {
pr_err("add: invalid interface name\n");
Thanks,
Kamal
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-23 19:45 ` Kamal Heib
@ 2020-08-24 6:23 ` Leon Romanovsky
2020-08-24 13:47 ` Jason Gunthorpe
1 sibling, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2020-08-24 6:23 UTC (permalink / raw)
To: Kamal Heib; +Cc: Jason Gunthorpe, linux-rdma, Doug Ledford, Zhu Yanjun
On Sun, Aug 23, 2020 at 10:45:58PM +0300, Kamal Heib wrote:
> On Thu, Aug 20, 2020 at 08:37:17AM -0300, Jason Gunthorpe wrote:
> > On Wed, Aug 19, 2020 at 12:15:45AM +0300, Kamal Heib wrote:
> > > On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> > > > On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > > > > To avoid the following kernel panic when calling kmem_cache_create()
> > > > > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > > > > context of device initialization.
> > > >
> > > > I think you've hit on a bigger bug than just this oops.
> > > >
> > > > rxe_net_add() should never be called before rxe_module_init(), that
> > > > surely subtly breaks all kinds of things.
> > > >
> > > > Maybe it is time to remove these module parameters?
> > > >
> > > Yes, I agree, this can be done in for-next.
> > >
> > > But at least can we take this patch to for-rc (stable) to fix this issue
> > > in stable releases?
> >
> > If you want to fix something in stable then block the module options
> > from working as actual module options - eg before rxe_module_init()
> > runs.
> >
> > Jason
>
> Something like the following patch?
If you want to got this path, it will be more like this: request_module("rdma_rxe")
Thanks
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 907203afbd99..872ebc57ac06 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -40,6 +40,8 @@ MODULE_AUTHOR("Bob Pearson, Frank Zago, John Groves, Kamal Heib");
> MODULE_DESCRIPTION("Soft RDMA transport");
> MODULE_LICENSE("Dual BSD/GPL");
>
> +bool rxe_is_loaded = false;
> +
> /* free resources for a rxe device all objects created for this device must
> * have been destroyed
> */
> @@ -315,6 +317,7 @@ static int __init rxe_module_init(void)
> return err;
>
> rdma_link_register(&rxe_link_ops);
> + rxe_is_loaded = true;
> pr_info("loaded\n");
> return 0;
> }
> @@ -326,6 +329,7 @@ static void __exit rxe_module_exit(void)
> rxe_net_exit();
> rxe_cache_exit();
>
> + rxe_is_loaded = false;
> pr_info("unloaded\n");
> }
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index fb07eed9e402..d9b71b5e2fba 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -67,6 +67,8 @@
>
> #define RXE_ROCE_V2_SPORT (0xc000)
>
> +extern bool rxe_is_loaded;
> +
> static inline u32 rxe_crc32(struct rxe_dev *rxe,
> u32 crc, void *next, size_t len)
> {
> diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> index ccda5f5a3bc0..12c7ca0764d5 100644
> --- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
> +++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
> @@ -61,6 +61,11 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
> struct net_device *ndev;
> struct rxe_dev *exists;
>
> + if (!rxe_is_loaded) {
> + pr_err("Please make sure to load the rdma_rxe module first\n");
> + return -EINVAL;
> + }
> +
> len = sanitize_arg(val, intf, sizeof(intf));
> if (!len) {
> pr_err("add: invalid interface name\n");
>
> Thanks,
> Kamal
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-23 19:45 ` Kamal Heib
2020-08-24 6:23 ` Leon Romanovsky
@ 2020-08-24 13:47 ` Jason Gunthorpe
2020-08-24 15:34 ` Kamal Heib
1 sibling, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2020-08-24 13:47 UTC (permalink / raw)
To: Kamal Heib; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Sun, Aug 23, 2020 at 10:45:58PM +0300, Kamal Heib wrote:
> On Thu, Aug 20, 2020 at 08:37:17AM -0300, Jason Gunthorpe wrote:
> > On Wed, Aug 19, 2020 at 12:15:45AM +0300, Kamal Heib wrote:
> > > On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> > > > On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > > > > To avoid the following kernel panic when calling kmem_cache_create()
> > > > > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > > > > context of device initialization.
> > > >
> > > > I think you've hit on a bigger bug than just this oops.
> > > >
> > > > rxe_net_add() should never be called before rxe_module_init(), that
> > > > surely subtly breaks all kinds of things.
> > > >
> > > > Maybe it is time to remove these module parameters?
> > > >
> > > Yes, I agree, this can be done in for-next.
> > >
> > > But at least can we take this patch to for-rc (stable) to fix this issue
> > > in stable releases?
> >
> > If you want to fix something in stable then block the module options
> > from working as actual module options - eg before rxe_module_init()
> > runs.
> >
> > Jason
>
> Something like the following patch?
Yes, something more like that
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create()
2020-08-24 13:47 ` Jason Gunthorpe
@ 2020-08-24 15:34 ` Kamal Heib
0 siblings, 0 replies; 8+ messages in thread
From: Kamal Heib @ 2020-08-24 15:34 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma, Doug Ledford, Leon Romanovsky, Zhu Yanjun
On Mon, Aug 24, 2020 at 10:47:23AM -0300, Jason Gunthorpe wrote:
> On Sun, Aug 23, 2020 at 10:45:58PM +0300, Kamal Heib wrote:
> > On Thu, Aug 20, 2020 at 08:37:17AM -0300, Jason Gunthorpe wrote:
> > > On Wed, Aug 19, 2020 at 12:15:45AM +0300, Kamal Heib wrote:
> > > > On Tue, Aug 18, 2020 at 01:31:57PM -0300, Jason Gunthorpe wrote:
> > > > > On Tue, Aug 18, 2020 at 05:25:04PM +0300, Kamal Heib wrote:
> > > > > > To avoid the following kernel panic when calling kmem_cache_create()
> > > > > > with a NULL pointer from pool_cache(), move the rxe_cache_init() to the
> > > > > > context of device initialization.
> > > > >
> > > > > I think you've hit on a bigger bug than just this oops.
> > > > >
> > > > > rxe_net_add() should never be called before rxe_module_init(), that
> > > > > surely subtly breaks all kinds of things.
> > > > >
> > > > > Maybe it is time to remove these module parameters?
> > > > >
> > > > Yes, I agree, this can be done in for-next.
> > > >
> > > > But at least can we take this patch to for-rc (stable) to fix this issue
> > > > in stable releases?
> > >
> > > If you want to fix something in stable then block the module options
> > > from working as actual module options - eg before rxe_module_init()
> > > runs.
> > >
> > > Jason
> >
> > Something like the following patch?
>
> Yes, something more like that
>
> Jason
OK, Thanks!
I'll send v3 soon.
Thanks,
Kamal
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-24 15:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-18 14:25 [PATCH v2 for-rc] RDMA/rxe: Fix panic when calling kmem_cache_create() Kamal Heib
2020-08-18 16:31 ` Jason Gunthorpe
2020-08-18 21:15 ` Kamal Heib
2020-08-20 11:37 ` Jason Gunthorpe
2020-08-23 19:45 ` Kamal Heib
2020-08-24 6:23 ` Leon Romanovsky
2020-08-24 13:47 ` Jason Gunthorpe
2020-08-24 15:34 ` Kamal Heib
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).