* [bug report] IB/mlx4: Add support for WQ related verbs
@ 2017-08-01 21:41 Dan Carpenter
2017-08-02 6:43 ` Leon Romanovsky
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-08-01 21:41 UTC (permalink / raw)
To: guyle-VPRAkNaXOzVWk0Htik3J/w; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Guy Levi,
This is a semi-automatic email about new static checker warnings.
The patch 400b1ebcfe31: "IB/mlx4: Add support for WQ related verbs"
from Jul 4, 2017, leads to the following Smatch complaint:
drivers/infiniband/hw/mlx4/qp.c:1203 create_qp_common()
error: we previously assumed 'pd->uobject' could be null (see line 1033)
drivers/infiniband/hw/mlx4/qp.c
1032
1033 if (pd->uobject) {
^^^^^^^^^^^
New check for NULL.
1034 union {
1035 struct mlx4_ib_create_qp qp;
1036 struct mlx4_ib_create_wq wq;
1037 } ucmd;
1038 size_t copy_len;
1039
1040 copy_len = (src == MLX4_IB_QP_SRC) ?
1041 sizeof(struct mlx4_ib_create_qp) :
1042 min(sizeof(struct mlx4_ib_create_wq), udata->inlen);
1043
1044 if (ib_copy_from_udata(&ucmd, udata, copy_len)) {
1045 err = -EFAULT;
1046 goto err;
1047 }
1048
1049 if (src == MLX4_IB_RWQ_SRC) {
1050 if (ucmd.wq.comp_mask || ucmd.wq.reserved1 ||
1051 ucmd.wq.reserved[0] || ucmd.wq.reserved[1] ||
1052 ucmd.wq.reserved[2]) {
1053 pr_debug("user command isn't supported\n");
1054 err = -EOPNOTSUPP;
1055 goto err;
1056 }
1057
1058 if (ucmd.wq.log_range_size >
1059 ilog2(dev->dev->caps.max_rss_tbl_sz)) {
1060 pr_debug("WQN range size must be equal or smaller than %d\n",
1061 dev->dev->caps.max_rss_tbl_sz);
1062 err = -EOPNOTSUPP;
1063 goto err;
1064 }
1065 range_size = 1 << ucmd.wq.log_range_size;
1066 } else {
1067 qp->inl_recv_sz = ucmd.qp.inl_recv_sz;
1068 }
1069
1070 err = set_rq_size(dev, &init_attr->cap, !!pd->uobject,
^^^^^^^^^^^
Not necessary check for NULL.
1071 qp_has_rq(init_attr), qp, qp->inl_recv_sz);
1072 if (err)
1073 goto err;
1074
1075 if (src == MLX4_IB_QP_SRC) {
1076 qp->sq_no_prefetch = ucmd.qp.sq_no_prefetch;
1077
1078 err = set_user_sq_size(dev, qp,
1079 (struct mlx4_ib_create_qp *)
1080 &ucmd);
1081 if (err)
1082 goto err;
1083 } else {
1084 qp->sq_no_prefetch = 1;
1085 qp->sq.wqe_cnt = 1;
1086 qp->sq.wqe_shift = MLX4_IB_MIN_SQ_STRIDE;
1087 /* Allocated buffer expects to have at least that SQ
1088 * size.
1089 */
1090 qp->buf_size = (qp->rq.wqe_cnt << qp->rq.wqe_shift) +
1091 (qp->sq.wqe_cnt << qp->sq.wqe_shift);
1092 }
1093
1094 qp->umem = ib_umem_get(pd->uobject->context,
1095 (src == MLX4_IB_QP_SRC) ? ucmd.qp.buf_addr :
1096 ucmd.wq.buf_addr, qp->buf_size, 0, 0);
1097 if (IS_ERR(qp->umem)) {
1098 err = PTR_ERR(qp->umem);
1099 goto err;
1100 }
1101
1102 err = mlx4_mtt_init(dev->dev, ib_umem_page_count(qp->umem),
1103 qp->umem->page_shift, &qp->mtt);
1104 if (err)
1105 goto err_buf;
1106
1107 err = mlx4_ib_umem_write_mtt(dev, &qp->mtt, qp->umem);
1108 if (err)
1109 goto err_mtt;
1110
1111 if (qp_has_rq(init_attr)) {
1112 err = mlx4_ib_db_map_user(to_mucontext(pd->uobject->context),
1113 (src == MLX4_IB_QP_SRC) ? ucmd.qp.db_addr :
1114 ucmd.wq.db_addr, &qp->db);
1115 if (err)
1116 goto err_mtt;
1117 }
1118 qp->mqp.usage = MLX4_RES_USAGE_USER_VERBS;
1119 } else {
1120 err = set_rq_size(dev, &init_attr->cap, !!pd->uobject,
^^^^^^^^^^^^^
Always NULL now.
1121 qp_has_rq(init_attr), qp, 0);
1122 if (err)
1123 goto err;
1124
1125 qp->sq_no_prefetch = 0;
1126
1127 if (init_attr->create_flags & IB_QP_CREATE_IPOIB_UD_LSO)
1128 qp->flags |= MLX4_IB_QP_LSO;
1129
1130 if (init_attr->create_flags & IB_QP_CREATE_NETIF_QP) {
1131 if (dev->steering_support ==
1132 MLX4_STEERING_MODE_DEVICE_MANAGED)
1133 qp->flags |= MLX4_IB_QP_NETIF;
1134 else
1135 goto err;
1136 }
1137
1138 memcpy(&backup_cap, &init_attr->cap, sizeof(backup_cap));
1139 err = set_kernel_sq_size(dev, &init_attr->cap,
1140 qp_type, qp, true);
1141 if (err)
1142 goto err;
1143
1144 if (qp_has_rq(init_attr)) {
1145 err = mlx4_db_alloc(dev->dev, &qp->db, 0);
1146 if (err)
1147 goto err;
1148
1149 *qp->db.db = 0;
1150 }
1151
1152 if (mlx4_buf_alloc(dev->dev, qp->buf_size, qp->buf_size,
1153 &qp->buf)) {
1154 memcpy(&init_attr->cap, &backup_cap,
1155 sizeof(backup_cap));
1156 err = set_kernel_sq_size(dev, &init_attr->cap, qp_type,
1157 qp, false);
1158 if (err)
1159 goto err_db;
1160
1161 if (mlx4_buf_alloc(dev->dev, qp->buf_size,
1162 PAGE_SIZE * 2, &qp->buf)) {
1163 err = -ENOMEM;
1164 goto err_db;
1165 }
1166 }
1167
1168 err = mlx4_mtt_init(dev->dev, qp->buf.npages, qp->buf.page_shift,
1169 &qp->mtt);
1170 if (err)
1171 goto err_buf;
1172
1173 err = mlx4_buf_write_mtt(dev->dev, &qp->mtt, &qp->buf);
1174 if (err)
1175 goto err_mtt;
1176
1177 qp->sq.wrid = kmalloc_array(qp->sq.wqe_cnt, sizeof(u64),
1178 GFP_KERNEL | __GFP_NOWARN);
1179 if (!qp->sq.wrid)
1180 qp->sq.wrid = __vmalloc(qp->sq.wqe_cnt * sizeof(u64),
1181 GFP_KERNEL, PAGE_KERNEL);
1182 qp->rq.wrid = kmalloc_array(qp->rq.wqe_cnt, sizeof(u64),
1183 GFP_KERNEL | __GFP_NOWARN);
1184 if (!qp->rq.wrid)
1185 qp->rq.wrid = __vmalloc(qp->rq.wqe_cnt * sizeof(u64),
1186 GFP_KERNEL, PAGE_KERNEL);
1187 if (!qp->sq.wrid || !qp->rq.wrid) {
1188 err = -ENOMEM;
1189 goto err_wrid;
1190 }
1191 qp->mqp.usage = MLX4_RES_USAGE_DRIVER;
1192 }
1193
1194 if (sqpn) {
1195 if (qp->mlx4_ib_qp_type & (MLX4_IB_QPT_PROXY_SMI_OWNER |
1196 MLX4_IB_QPT_PROXY_SMI | MLX4_IB_QPT_PROXY_GSI)) {
1197 if (alloc_proxy_bufs(pd->device, qp)) {
1198 err = -ENOMEM;
1199 goto err_wrid;
1200 }
1201 }
1202 } else if (src == MLX4_IB_RWQ_SRC) {
1203 err = mlx4_ib_alloc_wqn(to_mucontext(pd->uobject->context), qp,
^^^^^^^^^^^^^^^^^^^^
Unchecked dereference.
1204 range_size, &qpn);
1205 if (err)
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] IB/mlx4: Add support for WQ related verbs
2017-08-01 21:41 [bug report] IB/mlx4: Add support for WQ related verbs Dan Carpenter
@ 2017-08-02 6:43 ` Leon Romanovsky
[not found] ` <20170802064314.GY13672-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Leon Romanovsky @ 2017-08-02 6:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: guyle-VPRAkNaXOzVWk0Htik3J/w, linux-rdma-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 7793 bytes --]
On Wed, Aug 02, 2017 at 12:41:09AM +0300, Dan Carpenter wrote:
> Hello Guy Levi,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 400b1ebcfe31: "IB/mlx4: Add support for WQ related verbs"
> from Jul 4, 2017, leads to the following Smatch complaint:
>
> drivers/infiniband/hw/mlx4/qp.c:1203 create_qp_common()
> error: we previously assumed 'pd->uobject' could be null (see line 1033)
>
Thanks Dan,
As far as I can tell, this is false alarm,
The entry to the following call
1203 err = mlx4_ib_alloc_wqn(to_mucontext(pd->uobject->context), qp,
^^^^^^^^^^^^^^^^^^^^
is limited by
1202 } else if (src == MLX4_IB_RWQ_SRC) {
The src is set in mlx4_ib_create_wq
4201 err = create_qp_common(dev, pd, MLX4_IB_RWQ_SRC, &ib_qp_init_attr,
4202 udata, 0, &qp);
The mlx4_ib_create_wq has a check of pd->object at the beginning of it:
4153 if (!(udata && pd->uobject))
4154 return ERR_PTR(-EINVAL);
and it will ensure that pd-<object is valid.
> drivers/infiniband/hw/mlx4/qp.c
> 1032
> 1033 if (pd->uobject) {
> ^^^^^^^^^^^
> New check for NULL.
>
> 1034 union {
> 1035 struct mlx4_ib_create_qp qp;
> 1036 struct mlx4_ib_create_wq wq;
> 1037 } ucmd;
> 1038 size_t copy_len;
> 1039
> 1040 copy_len = (src == MLX4_IB_QP_SRC) ?
> 1041 sizeof(struct mlx4_ib_create_qp) :
> 1042 min(sizeof(struct mlx4_ib_create_wq), udata->inlen);
> 1043
> 1044 if (ib_copy_from_udata(&ucmd, udata, copy_len)) {
> 1045 err = -EFAULT;
> 1046 goto err;
> 1047 }
> 1048
> 1049 if (src == MLX4_IB_RWQ_SRC) {
> 1050 if (ucmd.wq.comp_mask || ucmd.wq.reserved1 ||
> 1051 ucmd.wq.reserved[0] || ucmd.wq.reserved[1] ||
> 1052 ucmd.wq.reserved[2]) {
> 1053 pr_debug("user command isn't supported\n");
> 1054 err = -EOPNOTSUPP;
> 1055 goto err;
> 1056 }
> 1057
> 1058 if (ucmd.wq.log_range_size >
> 1059 ilog2(dev->dev->caps.max_rss_tbl_sz)) {
> 1060 pr_debug("WQN range size must be equal or smaller than %d\n",
> 1061 dev->dev->caps.max_rss_tbl_sz);
> 1062 err = -EOPNOTSUPP;
> 1063 goto err;
> 1064 }
> 1065 range_size = 1 << ucmd.wq.log_range_size;
> 1066 } else {
> 1067 qp->inl_recv_sz = ucmd.qp.inl_recv_sz;
> 1068 }
> 1069
> 1070 err = set_rq_size(dev, &init_attr->cap, !!pd->uobject,
> ^^^^^^^^^^^
> Not necessary check for NULL.
>
> 1071 qp_has_rq(init_attr), qp, qp->inl_recv_sz);
> 1072 if (err)
> 1073 goto err;
> 1074
> 1075 if (src == MLX4_IB_QP_SRC) {
> 1076 qp->sq_no_prefetch = ucmd.qp.sq_no_prefetch;
> 1077
> 1078 err = set_user_sq_size(dev, qp,
> 1079 (struct mlx4_ib_create_qp *)
> 1080 &ucmd);
> 1081 if (err)
> 1082 goto err;
> 1083 } else {
> 1084 qp->sq_no_prefetch = 1;
> 1085 qp->sq.wqe_cnt = 1;
> 1086 qp->sq.wqe_shift = MLX4_IB_MIN_SQ_STRIDE;
> 1087 /* Allocated buffer expects to have at least that SQ
> 1088 * size.
> 1089 */
> 1090 qp->buf_size = (qp->rq.wqe_cnt << qp->rq.wqe_shift) +
> 1091 (qp->sq.wqe_cnt << qp->sq.wqe_shift);
> 1092 }
> 1093
> 1094 qp->umem = ib_umem_get(pd->uobject->context,
> 1095 (src == MLX4_IB_QP_SRC) ? ucmd.qp.buf_addr :
> 1096 ucmd.wq.buf_addr, qp->buf_size, 0, 0);
> 1097 if (IS_ERR(qp->umem)) {
> 1098 err = PTR_ERR(qp->umem);
> 1099 goto err;
> 1100 }
> 1101
> 1102 err = mlx4_mtt_init(dev->dev, ib_umem_page_count(qp->umem),
> 1103 qp->umem->page_shift, &qp->mtt);
> 1104 if (err)
> 1105 goto err_buf;
> 1106
> 1107 err = mlx4_ib_umem_write_mtt(dev, &qp->mtt, qp->umem);
> 1108 if (err)
> 1109 goto err_mtt;
> 1110
> 1111 if (qp_has_rq(init_attr)) {
> 1112 err = mlx4_ib_db_map_user(to_mucontext(pd->uobject->context),
> 1113 (src == MLX4_IB_QP_SRC) ? ucmd.qp.db_addr :
> 1114 ucmd.wq.db_addr, &qp->db);
> 1115 if (err)
> 1116 goto err_mtt;
> 1117 }
> 1118 qp->mqp.usage = MLX4_RES_USAGE_USER_VERBS;
> 1119 } else {
> 1120 err = set_rq_size(dev, &init_attr->cap, !!pd->uobject,
> ^^^^^^^^^^^^^
> Always NULL now.
>
> 1121 qp_has_rq(init_attr), qp, 0);
> 1122 if (err)
> 1123 goto err;
> 1124
> 1125 qp->sq_no_prefetch = 0;
> 1126
> 1127 if (init_attr->create_flags & IB_QP_CREATE_IPOIB_UD_LSO)
> 1128 qp->flags |= MLX4_IB_QP_LSO;
> 1129
> 1130 if (init_attr->create_flags & IB_QP_CREATE_NETIF_QP) {
> 1131 if (dev->steering_support ==
> 1132 MLX4_STEERING_MODE_DEVICE_MANAGED)
> 1133 qp->flags |= MLX4_IB_QP_NETIF;
> 1134 else
> 1135 goto err;
> 1136 }
> 1137
> 1138 memcpy(&backup_cap, &init_attr->cap, sizeof(backup_cap));
> 1139 err = set_kernel_sq_size(dev, &init_attr->cap,
> 1140 qp_type, qp, true);
> 1141 if (err)
> 1142 goto err;
> 1143
> 1144 if (qp_has_rq(init_attr)) {
> 1145 err = mlx4_db_alloc(dev->dev, &qp->db, 0);
> 1146 if (err)
> 1147 goto err;
> 1148
> 1149 *qp->db.db = 0;
> 1150 }
> 1151
> 1152 if (mlx4_buf_alloc(dev->dev, qp->buf_size, qp->buf_size,
> 1153 &qp->buf)) {
> 1154 memcpy(&init_attr->cap, &backup_cap,
> 1155 sizeof(backup_cap));
> 1156 err = set_kernel_sq_size(dev, &init_attr->cap, qp_type,
> 1157 qp, false);
> 1158 if (err)
> 1159 goto err_db;
> 1160
> 1161 if (mlx4_buf_alloc(dev->dev, qp->buf_size,
> 1162 PAGE_SIZE * 2, &qp->buf)) {
> 1163 err = -ENOMEM;
> 1164 goto err_db;
> 1165 }
> 1166 }
> 1167
> 1168 err = mlx4_mtt_init(dev->dev, qp->buf.npages, qp->buf.page_shift,
> 1169 &qp->mtt);
> 1170 if (err)
> 1171 goto err_buf;
> 1172
> 1173 err = mlx4_buf_write_mtt(dev->dev, &qp->mtt, &qp->buf);
> 1174 if (err)
> 1175 goto err_mtt;
> 1176
> 1177 qp->sq.wrid = kmalloc_array(qp->sq.wqe_cnt, sizeof(u64),
> 1178 GFP_KERNEL | __GFP_NOWARN);
> 1179 if (!qp->sq.wrid)
> 1180 qp->sq.wrid = __vmalloc(qp->sq.wqe_cnt * sizeof(u64),
> 1181 GFP_KERNEL, PAGE_KERNEL);
> 1182 qp->rq.wrid = kmalloc_array(qp->rq.wqe_cnt, sizeof(u64),
> 1183 GFP_KERNEL | __GFP_NOWARN);
> 1184 if (!qp->rq.wrid)
> 1185 qp->rq.wrid = __vmalloc(qp->rq.wqe_cnt * sizeof(u64),
> 1186 GFP_KERNEL, PAGE_KERNEL);
> 1187 if (!qp->sq.wrid || !qp->rq.wrid) {
> 1188 err = -ENOMEM;
> 1189 goto err_wrid;
> 1190 }
> 1191 qp->mqp.usage = MLX4_RES_USAGE_DRIVER;
> 1192 }
> 1193
> 1194 if (sqpn) {
> 1195 if (qp->mlx4_ib_qp_type & (MLX4_IB_QPT_PROXY_SMI_OWNER |
> 1196 MLX4_IB_QPT_PROXY_SMI | MLX4_IB_QPT_PROXY_GSI)) {
> 1197 if (alloc_proxy_bufs(pd->device, qp)) {
> 1198 err = -ENOMEM;
> 1199 goto err_wrid;
> 1200 }
> 1201 }
> 1202 } else if (src == MLX4_IB_RWQ_SRC) {
> 1203 err = mlx4_ib_alloc_wqn(to_mucontext(pd->uobject->context), qp,
> ^^^^^^^^^^^^^^^^^^^^
> Unchecked dereference.
>
> 1204 range_size, &qpn);
> 1205 if (err)
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] IB/mlx4: Add support for WQ related verbs
[not found] ` <20170802064314.GY13672-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2017-08-02 9:48 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-08-02 9:48 UTC (permalink / raw)
To: Leon Romanovsky
Cc: guyle-VPRAkNaXOzVWk0Htik3J/w, linux-rdma-u79uwXL29TY76Z2rM5mHXA
Yeah. You're right.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-08-02 9:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-01 21:41 [bug report] IB/mlx4: Add support for WQ related verbs Dan Carpenter
2017-08-02 6:43 ` Leon Romanovsky
[not found] ` <20170802064314.GY13672-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2017-08-02 9:48 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox