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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 2 Sep 2019 20:58:03 -0500
From:   "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To:     Zhu Yanjun <yanjun.zhu@...cle.com>, santosh.shilimkar@...cle.com,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, rds-devel@....oracle.com,
        gerd.rausch@...cle.com
Subject: Re: [PATCHv2 1/1] net: rds: add service level support in rds-info

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.

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


--
Gustavo