Re: [PATCHv2 1/1] net: rds: add service level support in rds-info

[Zhu Yanjun]

On 2019/9/3 9:58, Gustavo A. R. Silva wrote:

Hi,

On 8/23/19 8:04 PM, Zhu Yanjun wrote:

[..]

diff --git a/net/rds/ib.c b/net/rds/ib.c
index ec05d91..45acab2 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -291,7 +291,7 @@ static int rds_ib_conn_info_visitor(struct rds_connection 
*conn,
                                    void *buffer)
  {
        struct rds_info_rdma_connection *iinfo = buffer;
-       struct rds_ib_connection *ic;
+       struct rds_ib_connection *ic = conn->c_transport_data;

/* We will only ever look at IB transports */

        if (conn->c_trans != &rds_ib_transport)
@@ -301,15 +301,16 @@ static int rds_ib_conn_info_visitor(struct rds_connection 
*conn,

iinfo->src_addr = conn->c_laddr.s6_addr32[3];

        iinfo->dst_addr = conn->c_faddr.s6_addr32[3];
-       iinfo->tos = conn->c_tos;
+       if (ic) {

Is this null-check actually necessary? (see related comments below...)

+               iinfo->tos = conn->c_tos;
+               iinfo->sl = ic->i_sl;
+       }

memset(&iinfo->src_gid, 0, sizeof(iinfo->src_gid));

        memset(&iinfo->dst_gid, 0, sizeof(iinfo->dst_gid));
        if (rds_conn_state(conn) == RDS_CONN_UP) {
                struct rds_ib_device *rds_ibdev;

- ic = conn->c_transport_data;

-
                rdma_read_gids(ic->i_cm_id, (union ib_gid *)&iinfo->src_gid,

Notice that *ic* is dereferenced here without null-checking it. More
comments below...

                               (union ib_gid *)&iinfo->dst_gid);

@@ -329,7 +330,7 @@ static int rds6_ib_conn_info_visitor(struct rds_connection *conn,

                                     void *buffer)
  {
        struct rds6_info_rdma_connection *iinfo6 = buffer;
-       struct rds_ib_connection *ic;
+       struct rds_ib_connection *ic = conn->c_transport_data;

/* We will only ever look at IB transports */

        if (conn->c_trans != &rds_ib_transport)
@@ -337,6 +338,10 @@ static int rds6_ib_conn_info_visitor(struct rds_connection 
*conn,

iinfo6->src_addr = conn->c_laddr;

        iinfo6->dst_addr = conn->c_faddr;
+       if (ic) {
+               iinfo6->tos = conn->c_tos;
+               iinfo6->sl = ic->i_sl;
+       }

memset(&iinfo6->src_gid, 0, sizeof(iinfo6->src_gid));

        memset(&iinfo6->dst_gid, 0, sizeof(iinfo6->dst_gid));
@@ -344,7 +349,6 @@ static int rds6_ib_conn_info_visitor(struct rds_connection 
*conn,
        if (rds_conn_state(conn) == RDS_CONN_UP) {
                struct rds_ib_device *rds_ibdev;

- ic = conn->c_transport_data;

                rdma_read_gids(ic->i_cm_id, (union ib_gid *)&iinfo6->src_gid,

Again, *ic* is being dereferenced here without a previous null-check.

Please  check when this "rds_conn_state(conn) = RDS_CONN_UP".

Thanks a lot.

Zhu Yanjun

                               (union ib_gid *)&iinfo6->dst_gid);
                rds_ibdev = ic->rds_ibdev;

--
Gustavo