On Wed, Aug 5, 2020 at 11:16 AM Leon Romanovsky [off-list ref] wrote:

On Wed, Aug 05, 2020 at 11:09:00AM +0200, Danil Kipnis wrote:

quoted

Hi Leon,

On Wed, Aug 5, 2020 at 7:57 AM Leon Romanovsky [off-list ref] wrote:

quoted

On Tue, Aug 04, 2020 at 07:07:58PM +0530, Md Haris Iqbal wrote:

quoted

The rnbd_server module's communication manager (cm) initialization depends
on the registration of the "network namespace subsystem" of the RDMA CM
agent module. As such, when the kernel is configured to load the
rnbd_server and the RDMA cma module during initialization; and if the
rnbd_server module is initialized before RDMA cma module, a null ptr
dereference occurs during the RDMA bind operation.

Call trace below,

[    1.904782] Call Trace:
[    1.904782]  ? xas_load+0xd/0x80
[    1.904782]  xa_load+0x47/0x80
[    1.904782]  cma_ps_find+0x44/0x70
[    1.904782]  rdma_bind_addr+0x782/0x8b0
[    1.904782]  ? get_random_bytes+0x35/0x40
[    1.904782]  rtrs_srv_cm_init+0x50/0x80
[    1.904782]  rtrs_srv_open+0x102/0x180
[    1.904782]  ? rnbd_client_init+0x6e/0x6e
[    1.904782]  rnbd_srv_init_module+0x34/0x84
[    1.904782]  ? rnbd_client_init+0x6e/0x6e
[    1.904782]  do_one_initcall+0x4a/0x200
[    1.904782]  kernel_init_freeable+0x1f1/0x26e
[    1.904782]  ? rest_init+0xb0/0xb0
[    1.904782]  kernel_init+0xe/0x100
[    1.904782]  ret_from_fork+0x22/0x30
[    1.904782] Modules linked in:
[    1.904782] CR2: 0000000000000015
[    1.904782] ---[ end trace c42df88d6c7b0a48 ]---

All this happens cause the cm init is in the call chain of the module init,
which is not a preferred practice.

So remove the call to rdma_create_id() from the module init call chain.
Instead register rtrs-srv as an ib client, which makes sure that the
rdma_create_id() is called only when an ib device is added.

Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
Reported-by: kernel test robot <redacted>
Signed-off-by: Md Haris Iqbal <redacted>
---
 drivers/infiniband/ulp/rtrs/rtrs-srv.c | 77 +++++++++++++++++++++++++-
 drivers/infiniband/ulp/rtrs/rtrs-srv.h |  7 +++
 2 files changed, 81 insertions(+), 3 deletions(-)

Please don't send vX patches as reply-to in "git send-email" command.

I thought vX + in-reply-to makes it clear that a new version of a
patch is proposed in response to a mail reporting a problem in the
first version. Why is that a bad idea?

It looks very messy in e-mail client. It is hard to follow and requires
multiple iterations to understand if the reply is on previous variant or
on new one.

See attached print screen or see it in lore, where thread view is used.
https://lore.kernel.org/linux-rdma/20200623172321.GC6578@ziepe.ca/T/#t (local)

I see, thanks. Just a related question: In this particular case the
commit message changed in the second version of the patch. Should one
still use v2 in that case, or should the second version be submitted
without the vX tag at all?

Thanks