]>
Commit | Line | Data |
---|---|---|
f70185e4 GKH |
1 | From 62fc3357e079a07a22465b9b6ef71bb6ea75ee4b Mon Sep 17 00:00:00 2001 |
2 | From: Mahmoud Adam <mngyadam@amazon.com> | |
3 | Date: Tue, 26 Mar 2024 16:31:33 +0100 | |
4 | Subject: net/rds: fix possible cp null dereference | |
5 | ||
6 | From: Mahmoud Adam <mngyadam@amazon.com> | |
7 | ||
8 | commit 62fc3357e079a07a22465b9b6ef71bb6ea75ee4b upstream. | |
9 | ||
10 | cp might be null, calling cp->cp_conn would produce null dereference | |
11 | ||
12 | [Simon Horman adds:] | |
13 | ||
14 | Analysis: | |
15 | ||
16 | * cp is a parameter of __rds_rdma_map and is not reassigned. | |
17 | ||
18 | * The following call-sites pass a NULL cp argument to __rds_rdma_map() | |
19 | ||
20 | - rds_get_mr() | |
21 | - rds_get_mr_for_dest | |
22 | ||
23 | * Prior to the code above, the following assumes that cp may be NULL | |
24 | (which is indicative, but could itself be unnecessary) | |
25 | ||
26 | trans_private = rs->rs_transport->get_mr( | |
27 | sg, nents, rs, &mr->r_key, cp ? cp->cp_conn : NULL, | |
28 | args->vec.addr, args->vec.bytes, | |
29 | need_odp ? ODP_ZEROBASED : ODP_NOT_NEEDED); | |
30 | ||
31 | * The code modified by this patch is guarded by IS_ERR(trans_private), | |
32 | where trans_private is assigned as per the previous point in this analysis. | |
33 | ||
34 | The only implementation of get_mr that I could locate is rds_ib_get_mr() | |
35 | which can return an ERR_PTR if the conn (4th) argument is NULL. | |
36 | ||
37 | * ret is set to PTR_ERR(trans_private). | |
38 | rds_ib_get_mr can return ERR_PTR(-ENODEV) if the conn (4th) argument is NULL. | |
39 | Thus ret may be -ENODEV in which case the code in question will execute. | |
40 | ||
41 | Conclusion: | |
42 | * cp may be NULL at the point where this patch adds a check; | |
43 | this patch does seem to address a possible bug | |
44 | ||
45 | Fixes: c055fc00c07b ("net/rds: fix WARNING in rds_conn_connect_if_down") | |
46 | Cc: stable@vger.kernel.org # v4.19+ | |
47 | Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> | |
48 | Reviewed-by: Simon Horman <horms@kernel.org> | |
49 | Link: https://lore.kernel.org/r/20240326153132.55580-1-mngyadam@amazon.com | |
50 | Signed-off-by: Jakub Kicinski <kuba@kernel.org> | |
51 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
52 | --- | |
53 | net/rds/rdma.c | 2 +- | |
54 | 1 file changed, 1 insertion(+), 1 deletion(-) | |
55 | ||
56 | --- a/net/rds/rdma.c | |
57 | +++ b/net/rds/rdma.c | |
58 | @@ -302,7 +302,7 @@ static int __rds_rdma_map(struct rds_soc | |
59 | } | |
60 | ret = PTR_ERR(trans_private); | |
61 | /* Trigger connection so that its ready for the next retry */ | |
62 | - if (ret == -ENODEV) | |
63 | + if (ret == -ENODEV && cp) | |
64 | rds_conn_connect_if_down(cp->cp_conn); | |
65 | goto out; | |
66 | } |