Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Merge branch 'net-enetc-fix-command-bd-ring-issues'

Wei Fang says:

====================
net: enetc: fix command BD ring issues

Currently, the implementation of command BD ring has two issues, one is
that the driver may obtain wrong consumer index of the ring, because the
driver does not mask out the SBE bit of the CIR value, so a wrong index
will be obtained when a SBE error ouccrs. The other one is that the DMA
buffer may be used after free. If netc_xmit_ntmp_cmd() times out and
returns an error, the pending command is not explicitly aborted, while
ntmp_free_data_mem() unconditionally frees the DMA buffer. If the buffer
has already been reallocated elsewhere, this may lead to silent memory
corruption. Because the hardware eventually processes the pending command
and perform a DMA write of the response to the physical address of the
freed buffer. So this patch set is to fix these two issues.
====================

Link: https://patch.msgid.link/20260415060833.2303846-1-wei.fang@nxp.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

+148 -102
+137 -94
drivers/net/ethernet/freescale/enetc/ntmp.c
··· 7 7 #include <linux/dma-mapping.h> 8 8 #include <linux/fsl/netc_global.h> 9 9 #include <linux/iopoll.h> 10 + #include <linux/vmalloc.h> 10 11 11 12 #include "ntmp_private.h" 12 13 ··· 43 42 if (!cbdr->addr_base) 44 43 return -ENOMEM; 45 44 45 + cbdr->swcbd = vcalloc(cbd_num, sizeof(struct netc_swcbd)); 46 + if (!cbdr->swcbd) { 47 + dma_free_coherent(dev, size, cbdr->addr_base, cbdr->dma_base); 48 + return -ENOMEM; 49 + } 50 + 46 51 cbdr->dma_size = size; 47 52 cbdr->bd_num = cbd_num; 48 53 cbdr->regs = *regs; ··· 59 52 cbdr->addr_base_align = PTR_ALIGN(cbdr->addr_base, 60 53 NTMP_BASE_ADDR_ALIGN); 61 54 62 - spin_lock_init(&cbdr->ring_lock); 55 + mutex_init(&cbdr->ring_lock); 63 56 64 57 cbdr->next_to_use = netc_read(cbdr->regs.pir); 65 - cbdr->next_to_clean = netc_read(cbdr->regs.cir); 58 + cbdr->next_to_clean = netc_read(cbdr->regs.cir) & NETC_CBDRCIR_INDEX; 66 59 67 60 /* Step 1: Configure the base address of the Control BD Ring */ 68 61 netc_write(cbdr->regs.bar0, lower_32_bits(cbdr->dma_base_align)); ··· 78 71 } 79 72 EXPORT_SYMBOL_GPL(ntmp_init_cbdr); 80 73 74 + static void ntmp_free_data_mem(struct device *dev, struct netc_swcbd *swcbd) 75 + { 76 + if (unlikely(!swcbd->buf)) 77 + return; 78 + 79 + dma_free_coherent(dev, swcbd->size + NTMP_DATA_ADDR_ALIGN, 80 + swcbd->buf, swcbd->dma); 81 + } 82 + 81 83 void ntmp_free_cbdr(struct netc_cbdr *cbdr) 82 84 { 83 85 /* Disable the Control BD Ring */ 84 86 netc_write(cbdr->regs.mr, 0); 87 + 88 + for (int i = 0; i < cbdr->bd_num; i++) 89 + ntmp_free_data_mem(cbdr->dev, &cbdr->swcbd[i]); 90 + 91 + vfree(cbdr->swcbd); 85 92 dma_free_coherent(cbdr->dev, cbdr->dma_size, cbdr->addr_base, 86 93 cbdr->dma_base); 87 94 memset(cbdr, 0, sizeof(*cbdr)); ··· 115 94 116 95 static void ntmp_clean_cbdr(struct netc_cbdr *cbdr) 117 96 { 118 - union netc_cbd *cbd; 119 - int i; 97 + int i = cbdr->next_to_clean; 120 98 121 - i = cbdr->next_to_clean; 122 - while (netc_read(cbdr->regs.cir) != i) { 123 - cbd = ntmp_get_cbd(cbdr, i); 99 + while ((netc_read(cbdr->regs.cir) & NETC_CBDRCIR_INDEX) != i) { 100 + union netc_cbd *cbd = ntmp_get_cbd(cbdr, i); 101 + struct netc_swcbd *swcbd = &cbdr->swcbd[i]; 102 + 103 + ntmp_free_data_mem(cbdr->dev, swcbd); 104 + memset(swcbd, 0, sizeof(*swcbd)); 124 105 memset(cbd, 0, sizeof(*cbd)); 125 106 i = (i + 1) % cbdr->bd_num; 126 107 } 127 108 109 + dma_wmb(); 128 110 cbdr->next_to_clean = i; 129 111 } 130 112 131 - static int netc_xmit_ntmp_cmd(struct ntmp_user *user, union netc_cbd *cbd) 113 + static void ntmp_select_and_lock_cbdr(struct ntmp_user *user, 114 + struct netc_cbdr **cbdr) 115 + { 116 + /* Currently only ENETC is supported, and it has only one command 117 + * BD ring. 118 + */ 119 + *cbdr = &user->ring[0]; 120 + 121 + mutex_lock(&(*cbdr)->ring_lock); 122 + } 123 + 124 + static void ntmp_unlock_cbdr(struct netc_cbdr *cbdr) 125 + { 126 + mutex_unlock(&cbdr->ring_lock); 127 + } 128 + 129 + static int netc_xmit_ntmp_cmd(struct netc_cbdr *cbdr, union netc_cbd *cbd, 130 + struct netc_swcbd *swcbd) 132 131 { 133 132 union netc_cbd *cur_cbd; 134 - struct netc_cbdr *cbdr; 135 - int i, err; 133 + int i, err, used_bds; 136 134 u16 status; 137 135 u32 val; 138 136 139 - /* Currently only i.MX95 ENETC is supported, and it only has one 140 - * command BD ring 141 - */ 142 - cbdr = &user->ring[0]; 143 - 144 - spin_lock_bh(&cbdr->ring_lock); 145 - 146 - if (unlikely(!ntmp_get_free_cbd_num(cbdr))) 137 + used_bds = cbdr->bd_num - ntmp_get_free_cbd_num(cbdr); 138 + if (unlikely(used_bds >= NETC_CBDR_CLEAN_WORK)) { 147 139 ntmp_clean_cbdr(cbdr); 140 + if (unlikely(!ntmp_get_free_cbd_num(cbdr))) { 141 + ntmp_free_data_mem(cbdr->dev, swcbd); 142 + return -EBUSY; 143 + } 144 + } 148 145 149 146 i = cbdr->next_to_use; 150 147 cur_cbd = ntmp_get_cbd(cbdr, i); 151 148 *cur_cbd = *cbd; 149 + cbdr->swcbd[i] = *swcbd; 152 150 dma_wmb(); 153 151 154 152 /* Update producer index of both software and hardware */ ··· 175 135 cbdr->next_to_use = i; 176 136 netc_write(cbdr->regs.pir, i); 177 137 178 - err = read_poll_timeout_atomic(netc_read, val, val == i, 179 - NETC_CBDR_DELAY_US, NETC_CBDR_TIMEOUT, 180 - true, cbdr->regs.cir); 138 + err = read_poll_timeout(netc_read, val, 139 + (val & NETC_CBDRCIR_INDEX) == i, 140 + NETC_CBDR_DELAY_US, NETC_CBDR_TIMEOUT, 141 + true, cbdr->regs.cir); 181 142 if (unlikely(err)) 182 - goto cbdr_unlock; 143 + return err; 144 + 145 + if (unlikely(val & NETC_CBDRCIR_SBE)) { 146 + dev_err(cbdr->dev, "Command BD system bus error\n"); 147 + return -EIO; 148 + } 183 149 184 150 dma_rmb(); 185 151 /* Get the writeback command BD, because the caller may need ··· 196 150 /* Check the writeback error status */ 197 151 status = le16_to_cpu(cbd->resp_hdr.error_rr) & NTMP_RESP_ERROR; 198 152 if (unlikely(status)) { 199 - err = -EIO; 200 - dev_err(user->dev, "Command BD error: 0x%04x\n", status); 153 + dev_err(cbdr->dev, "Command BD error: 0x%04x\n", status); 154 + return -EIO; 201 155 } 202 - 203 - ntmp_clean_cbdr(cbdr); 204 - dma_wmb(); 205 - 206 - cbdr_unlock: 207 - spin_unlock_bh(&cbdr->ring_lock); 208 - 209 - return err; 210 - } 211 - 212 - static int ntmp_alloc_data_mem(struct ntmp_dma_buf *data, void **buf_align) 213 - { 214 - void *buf; 215 - 216 - buf = dma_alloc_coherent(data->dev, data->size + NTMP_DATA_ADDR_ALIGN, 217 - &data->dma, GFP_KERNEL); 218 - if (!buf) 219 - return -ENOMEM; 220 - 221 - data->buf = buf; 222 - *buf_align = PTR_ALIGN(buf, NTMP_DATA_ADDR_ALIGN); 223 156 224 157 return 0; 225 158 } 226 159 227 - static void ntmp_free_data_mem(struct ntmp_dma_buf *data) 160 + static int ntmp_alloc_data_mem(struct device *dev, struct netc_swcbd *swcbd, 161 + void **buf_align) 228 162 { 229 - dma_free_coherent(data->dev, data->size + NTMP_DATA_ADDR_ALIGN, 230 - data->buf, data->dma); 163 + void *buf; 164 + 165 + buf = dma_alloc_coherent(dev, swcbd->size + NTMP_DATA_ADDR_ALIGN, 166 + &swcbd->dma, GFP_KERNEL); 167 + if (!buf) 168 + return -ENOMEM; 169 + 170 + swcbd->buf = buf; 171 + *buf_align = PTR_ALIGN(buf, NTMP_DATA_ADDR_ALIGN); 172 + 173 + return 0; 231 174 } 232 175 233 176 static void ntmp_fill_request_hdr(union netc_cbd *cbd, dma_addr_t dma, ··· 269 234 u8 tbl_ver, u32 entry_id, u32 req_len, 270 235 u32 resp_len) 271 236 { 272 - struct ntmp_dma_buf data = { 273 - .dev = user->dev, 237 + struct netc_swcbd swcbd = { 274 238 .size = max(req_len, resp_len), 275 239 }; 276 240 struct ntmp_req_by_eid *req; 241 + struct netc_cbdr *cbdr; 277 242 union netc_cbd cbd; 278 243 int err; 279 244 280 - err = ntmp_alloc_data_mem(&data, (void **)&req); 245 + err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req); 281 246 if (err) 282 247 return err; 283 248 284 249 ntmp_fill_crd_eid(req, tbl_ver, 0, 0, entry_id); 285 - ntmp_fill_request_hdr(&cbd, data.dma, NTMP_LEN(req_len, resp_len), 250 + ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(req_len, resp_len), 286 251 tbl_id, NTMP_CMD_DELETE, NTMP_AM_ENTRY_ID); 287 252 288 - err = netc_xmit_ntmp_cmd(user, &cbd); 253 + ntmp_select_and_lock_cbdr(user, &cbdr); 254 + err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd); 289 255 if (err) 290 256 dev_err(user->dev, 291 257 "Failed to delete entry 0x%x of %s, err: %pe", 292 258 entry_id, ntmp_table_name(tbl_id), ERR_PTR(err)); 293 - 294 - ntmp_free_data_mem(&data); 259 + ntmp_unlock_cbdr(cbdr); 295 260 296 261 return err; 297 262 } 298 263 299 - static int ntmp_query_entry_by_id(struct ntmp_user *user, int tbl_id, 300 - u32 len, struct ntmp_req_by_eid *req, 301 - dma_addr_t dma, bool compare_eid) 264 + static int ntmp_query_entry_by_id(struct netc_cbdr *cbdr, int tbl_id, 265 + struct ntmp_req_by_eid *req, 266 + struct netc_swcbd *swcbd, 267 + bool compare_eid) 302 268 { 269 + u32 len = NTMP_LEN(sizeof(*req), swcbd->size); 303 270 struct ntmp_cmn_resp_query *resp; 304 271 int cmd = NTMP_CMD_QUERY; 305 272 union netc_cbd cbd; ··· 313 276 cmd = NTMP_CMD_QU; 314 277 315 278 /* Request header */ 316 - ntmp_fill_request_hdr(&cbd, dma, len, tbl_id, cmd, NTMP_AM_ENTRY_ID); 317 - err = netc_xmit_ntmp_cmd(user, &cbd); 279 + ntmp_fill_request_hdr(&cbd, swcbd->dma, len, tbl_id, cmd, 280 + NTMP_AM_ENTRY_ID); 281 + err = netc_xmit_ntmp_cmd(cbdr, &cbd, swcbd); 318 282 if (err) { 319 - dev_err(user->dev, 283 + dev_err(cbdr->dev, 320 284 "Failed to query entry 0x%x of %s, err: %pe\n", 321 285 entry_id, ntmp_table_name(tbl_id), ERR_PTR(err)); 322 286 return err; ··· 331 293 332 294 resp = (struct ntmp_cmn_resp_query *)req; 333 295 if (unlikely(le32_to_cpu(resp->entry_id) != entry_id)) { 334 - dev_err(user->dev, 296 + dev_err(cbdr->dev, 335 297 "%s: query EID 0x%x doesn't match response EID 0x%x\n", 336 298 ntmp_table_name(tbl_id), entry_id, le32_to_cpu(resp->entry_id)); 337 299 return -EIO; ··· 343 305 int ntmp_maft_add_entry(struct ntmp_user *user, u32 entry_id, 344 306 struct maft_entry_data *maft) 345 307 { 346 - struct ntmp_dma_buf data = { 347 - .dev = user->dev, 308 + struct netc_swcbd swcbd = { 348 309 .size = sizeof(struct maft_req_add), 349 310 }; 350 311 struct maft_req_add *req; 312 + struct netc_cbdr *cbdr; 351 313 union netc_cbd cbd; 352 314 int err; 353 315 354 - err = ntmp_alloc_data_mem(&data, (void **)&req); 316 + err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req); 355 317 if (err) 356 318 return err; 357 319 ··· 360 322 req->keye = maft->keye; 361 323 req->cfge = maft->cfge; 362 324 363 - ntmp_fill_request_hdr(&cbd, data.dma, NTMP_LEN(data.size, 0), 325 + ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(swcbd.size, 0), 364 326 NTMP_MAFT_ID, NTMP_CMD_ADD, NTMP_AM_ENTRY_ID); 365 - err = netc_xmit_ntmp_cmd(user, &cbd); 327 + 328 + ntmp_select_and_lock_cbdr(user, &cbdr); 329 + err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd); 366 330 if (err) 367 331 dev_err(user->dev, "Failed to add MAFT entry 0x%x, err: %pe\n", 368 332 entry_id, ERR_PTR(err)); 369 - 370 - ntmp_free_data_mem(&data); 333 + ntmp_unlock_cbdr(cbdr); 371 334 372 335 return err; 373 336 } ··· 377 338 int ntmp_maft_query_entry(struct ntmp_user *user, u32 entry_id, 378 339 struct maft_entry_data *maft) 379 340 { 380 - struct ntmp_dma_buf data = { 381 - .dev = user->dev, 341 + struct netc_swcbd swcbd = { 382 342 .size = sizeof(struct maft_resp_query), 383 343 }; 384 344 struct maft_resp_query *resp; 385 345 struct ntmp_req_by_eid *req; 346 + struct netc_cbdr *cbdr; 386 347 int err; 387 348 388 - err = ntmp_alloc_data_mem(&data, (void **)&req); 349 + err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req); 389 350 if (err) 390 351 return err; 391 352 392 353 ntmp_fill_crd_eid(req, user->tbl.maft_ver, 0, 0, entry_id); 393 - err = ntmp_query_entry_by_id(user, NTMP_MAFT_ID, 394 - NTMP_LEN(sizeof(*req), data.size), 395 - req, data.dma, true); 354 + 355 + ntmp_select_and_lock_cbdr(user, &cbdr); 356 + err = ntmp_query_entry_by_id(cbdr, NTMP_MAFT_ID, req, &swcbd, true); 396 357 if (err) 397 - goto end; 358 + goto unlock_cbdr; 398 359 399 360 resp = (struct maft_resp_query *)req; 400 361 maft->keye = resp->keye; 401 362 maft->cfge = resp->cfge; 402 363 403 - end: 404 - ntmp_free_data_mem(&data); 364 + unlock_cbdr: 365 + ntmp_unlock_cbdr(cbdr); 405 366 406 367 return err; 407 368 } ··· 417 378 int ntmp_rsst_update_entry(struct ntmp_user *user, const u32 *table, 418 379 int count) 419 380 { 420 - struct ntmp_dma_buf data = {.dev = user->dev}; 421 381 struct rsst_req_update *req; 382 + struct netc_swcbd swcbd; 383 + struct netc_cbdr *cbdr; 422 384 union netc_cbd cbd; 423 385 int err, i; 424 386 ··· 427 387 /* HW only takes in a full 64 entry table */ 428 388 return -EINVAL; 429 389 430 - data.size = struct_size(req, groups, count); 431 - err = ntmp_alloc_data_mem(&data, (void **)&req); 390 + swcbd.size = struct_size(req, groups, count); 391 + err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req); 432 392 if (err) 433 393 return err; 434 394 ··· 438 398 for (i = 0; i < count; i++) 439 399 req->groups[i] = (u8)(table[i]); 440 400 441 - ntmp_fill_request_hdr(&cbd, data.dma, NTMP_LEN(data.size, 0), 401 + ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(swcbd.size, 0), 442 402 NTMP_RSST_ID, NTMP_CMD_UPDATE, NTMP_AM_ENTRY_ID); 443 403 444 - err = netc_xmit_ntmp_cmd(user, &cbd); 404 + ntmp_select_and_lock_cbdr(user, &cbdr); 405 + err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd); 445 406 if (err) 446 407 dev_err(user->dev, "Failed to update RSST entry, err: %pe\n", 447 408 ERR_PTR(err)); 448 - 449 - ntmp_free_data_mem(&data); 409 + ntmp_unlock_cbdr(cbdr); 450 410 451 411 return err; 452 412 } ··· 454 414 455 415 int ntmp_rsst_query_entry(struct ntmp_user *user, u32 *table, int count) 456 416 { 457 - struct ntmp_dma_buf data = {.dev = user->dev}; 458 417 struct ntmp_req_by_eid *req; 418 + struct netc_swcbd swcbd; 419 + struct netc_cbdr *cbdr; 459 420 union netc_cbd cbd; 460 421 int err, i; 461 422 u8 *group; ··· 465 424 /* HW only takes in a full 64 entry table */ 466 425 return -EINVAL; 467 426 468 - data.size = NTMP_ENTRY_ID_SIZE + RSST_STSE_DATA_SIZE(count) + 469 - RSST_CFGE_DATA_SIZE(count); 470 - err = ntmp_alloc_data_mem(&data, (void **)&req); 427 + swcbd.size = NTMP_ENTRY_ID_SIZE + RSST_STSE_DATA_SIZE(count) + 428 + RSST_CFGE_DATA_SIZE(count); 429 + err = ntmp_alloc_data_mem(user->dev, &swcbd, (void **)&req); 471 430 if (err) 472 431 return err; 473 432 474 433 /* Set the request data buffer */ 475 434 ntmp_fill_crd_eid(req, user->tbl.rsst_ver, 0, 0, 0); 476 - ntmp_fill_request_hdr(&cbd, data.dma, NTMP_LEN(sizeof(*req), data.size), 435 + ntmp_fill_request_hdr(&cbd, swcbd.dma, NTMP_LEN(sizeof(*req), swcbd.size), 477 436 NTMP_RSST_ID, NTMP_CMD_QUERY, NTMP_AM_ENTRY_ID); 478 - err = netc_xmit_ntmp_cmd(user, &cbd); 437 + 438 + ntmp_select_and_lock_cbdr(user, &cbdr); 439 + err = netc_xmit_ntmp_cmd(cbdr, &cbd, &swcbd); 479 440 if (err) { 480 441 dev_err(user->dev, "Failed to query RSST entry, err: %pe\n", 481 442 ERR_PTR(err)); 482 - goto end; 443 + goto unlock_cbdr; 483 444 } 484 445 485 446 group = (u8 *)req; ··· 489 446 for (i = 0; i < count; i++) 490 447 table[i] = group[i]; 491 448 492 - end: 493 - ntmp_free_data_mem(&data); 449 + unlock_cbdr: 450 + ntmp_unlock_cbdr(cbdr); 494 451 495 452 return err; 496 453 }
+3 -7
drivers/net/ethernet/freescale/enetc/ntmp_private.h
··· 12 12 13 13 #define NTMP_EID_REQ_LEN 8 14 14 #define NETC_CBDR_BD_NUM 256 15 + #define NETC_CBDRCIR_INDEX GENMASK(9, 0) 16 + #define NETC_CBDRCIR_SBE BIT(31) 17 + #define NETC_CBDR_CLEAN_WORK 16 15 18 16 19 union netc_cbd { 17 20 struct { ··· 55 52 #define NTMP_RESP_RR BIT(15) 56 53 __le32 resv1[4]; 57 54 } resp_hdr; /* NTMP Response Message Header Format */ 58 - }; 59 - 60 - struct ntmp_dma_buf { 61 - struct device *dev; 62 - size_t size; 63 - void *buf; 64 - dma_addr_t dma; 65 55 }; 66 56 67 57 struct ntmp_cmn_req_data {
+8 -1
include/linux/fsl/ntmp.h
··· 31 31 u8 rsst_ver; 32 32 }; 33 33 34 + struct netc_swcbd { 35 + void *buf; 36 + dma_addr_t dma; 37 + size_t size; 38 + }; 39 + 34 40 struct netc_cbdr { 35 41 struct device *dev; 36 42 struct netc_cbdr_regs regs; ··· 50 44 void *addr_base_align; 51 45 dma_addr_t dma_base; 52 46 dma_addr_t dma_base_align; 47 + struct netc_swcbd *swcbd; 53 48 54 49 /* Serialize the order of command BD ring */ 55 - spinlock_t ring_lock; 50 + struct mutex ring_lock; 56 51 }; 57 52 58 53 struct ntmp_user {