| Message ID | 20200529133855.146357-1-hare@suse.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | scsi_scan: handle REPORT LUN overflow | expand |
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next hch-configfs/for-next v5.7-rc7 next-20200529]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi_scan-handle-REPORT-LUN-overflow/20200531-083517
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: i386-randconfig-s001-20200531 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-243-gc100a7ab-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/scsi_scan.c:1302:42: sparse: sparse: incorrect type in initializer (different base types) @@ expected int alloc_flags @@ got restricted gfp_t @@
>> drivers/scsi/scsi_scan.c:1302:42: sparse: expected int alloc_flags
>> drivers/scsi/scsi_scan.c:1302:42: sparse: got restricted gfp_t
>> drivers/scsi/scsi_scan.c:1339:36: sparse: sparse: restricted gfp_t degrades to integer
>> drivers/scsi/scsi_scan.c:1339:47: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected restricted gfp_t [usertype] flags @@ got unsigned int @@
>> drivers/scsi/scsi_scan.c:1339:47: sparse: expected restricted gfp_t [usertype] flags
>> drivers/scsi/scsi_scan.c:1339:47: sparse: got unsigned int
drivers/scsi/scsi_scan.c:1411:50: sparse: sparse: restricted gfp_t degrades to integer
drivers/scsi/scsi_scan.c:1411:61: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected restricted gfp_t [usertype] flags @@ got unsigned int @@
drivers/scsi/scsi_scan.c:1411:61: sparse: expected restricted gfp_t [usertype] flags
drivers/scsi/scsi_scan.c:1411:61: sparse: got unsigned int
vim +1302 drivers/scsi/scsi_scan.c
1267
1268 /**
1269 * scsi_report_lun_scan - Scan using SCSI REPORT LUN results
1270 * @starget: which target
1271 * @bflags: Zero or a mix of BLIST_NOLUN, BLIST_REPORTLUN2, or BLIST_NOREPORTLUN
1272 * @rescan: nonzero if we can skip code only needed on first scan
1273 *
1274 * Description:
1275 * Fast scanning for modern (SCSI-3) devices by sending a REPORT LUN command.
1276 * Scan the resulting list of LUNs by calling scsi_probe_and_add_lun.
1277 *
1278 * If BLINK_REPORTLUN2 is set, scan a target that supports more than 8
1279 * LUNs even if it's older than SCSI-3.
1280 * If BLIST_NOREPORTLUN is set, return 1 always.
1281 * If BLIST_NOLUN is set, return 0 always.
1282 * If starget->no_report_luns is set, return 1 always.
1283 *
1284 * Return:
1285 * 0: scan completed (or no memory, so further scanning is futile)
1286 * 1: could not scan with REPORT LUN
1287 **/
1288 static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflags,
1289 enum scsi_scan_mode rescan)
1290 {
1291 unsigned char scsi_cmd[MAX_COMMAND_SIZE];
1292 unsigned int length;
1293 u64 lun;
1294 unsigned int num_luns;
1295 unsigned int retries;
1296 int result;
1297 struct scsi_lun *lunp, *lun_data;
1298 struct scsi_sense_hdr sshdr;
1299 struct scsi_device *sdev;
1300 struct Scsi_Host *shost = dev_to_shost(&starget->dev);
1301 int ret = 0, alloc_flags =
> 1302 shost->unchecked_isa_dma ? __GFP_DMA : 0;
1303
1304 /*
1305 * Only support SCSI-3 and up devices if BLIST_NOREPORTLUN is not set.
1306 * Also allow SCSI-2 if BLIST_REPORTLUN2 is set and host adapter does
1307 * support more than 8 LUNs.
1308 * Don't attempt if the target doesn't support REPORT LUNS.
1309 */
1310 if (bflags & BLIST_NOREPORTLUN)
1311 return 1;
1312 if (starget->scsi_level < SCSI_2 &&
1313 starget->scsi_level != SCSI_UNKNOWN)
1314 return 1;
1315 if (starget->scsi_level < SCSI_3 &&
1316 (!(bflags & BLIST_REPORTLUN2) || shost->max_lun <= 8))
1317 return 1;
1318 if (bflags & BLIST_NOLUN)
1319 return 0;
1320 if (starget->no_report_luns)
1321 return 1;
1322
1323 if (!(sdev = scsi_device_lookup_by_target(starget, 0))) {
1324 sdev = scsi_alloc_sdev(starget, 0, NULL);
1325 if (!sdev)
1326 return 0;
1327 if (scsi_device_get(sdev)) {
1328 __scsi_remove_device(sdev);
1329 return 0;
1330 }
1331 }
1332
1333 /*
1334 * Allocate enough to hold the header (the same size as one scsi_lun)
1335 * plus the number of luns we are requesting. 511 was the default
1336 * value of the now removed max_report_luns parameter.
1337 */
1338 length = (511 + 1) * sizeof(struct scsi_lun);
> 1339 lun_data = kmalloc(length, GFP_KERNEL | alloc_flags);
1340 if (!lun_data) {
1341 printk(ALLOC_FAILURE_MSG, __func__);
1342 goto out;
1343 }
1344
1345 retry:
1346 scsi_cmd[0] = REPORT_LUNS;
1347
1348 /*
1349 * bytes 1 - 5: reserved, set to zero.
1350 */
1351 memset(&scsi_cmd[1], 0, 5);
1352
1353 /*
1354 * bytes 6 - 9: length of the command.
1355 */
1356 put_unaligned_be32(length, &scsi_cmd[6]);
1357
1358 scsi_cmd[10] = 0; /* reserved */
1359 scsi_cmd[11] = 0; /* control */
1360
1361 /*
1362 * We can get a UNIT ATTENTION, for example a power on/reset, so
1363 * retry a few times (like sd.c does for TEST UNIT READY).
1364 * Experience shows some combinations of adapter/devices get at
1365 * least two power on/resets.
1366 *
1367 * Illegal requests (for devices that do not support REPORT LUNS)
1368 * should come through as a check condition, and will not generate
1369 * a retry.
1370 */
1371 for (retries = 0; retries < 3; retries++) {
1372 SCSI_LOG_SCAN_BUS(3, sdev_printk (KERN_INFO, sdev,
1373 "scsi scan: Sending REPORT LUNS to (try %d)\n",
1374 retries));
1375
1376 result = scsi_execute_req(sdev, scsi_cmd, DMA_FROM_DEVICE,
1377 lun_data, length, &sshdr,
1378 SCSI_REPORT_LUNS_TIMEOUT, 3, NULL);
1379
1380 SCSI_LOG_SCAN_BUS(3, sdev_printk (KERN_INFO, sdev,
1381 "scsi scan: REPORT LUNS"
1382 " %s (try %d) result 0x%x\n",
1383 result ? "failed" : "successful",
1384 retries, result));
1385 if (result == 0)
1386 break;
1387 else if (scsi_sense_valid(&sshdr)) {
1388 if (sshdr.sense_key != UNIT_ATTENTION)
1389 break;
1390 }
1391 }
1392
1393 if (result) {
1394 /*
1395 * The device probably does not support a REPORT LUN command
1396 */
1397 ret = 1;
1398 goto out_err;
1399 }
1400
1401 /*
1402 * Get the length from the first four bytes of lun_data.
1403 */
1404 if (get_unaligned_be32(lun_data->scsi_lun) +
1405 sizeof(struct scsi_lun) > length) {
1406 unsigned int resp_length;
1407 void *resp_data;
1408
1409 resp_length = get_unaligned_be32(lun_data->scsi_lun) +
1410 sizeof(struct scsi_lun);
1411 resp_data = kmalloc(resp_length, GFP_KERNEL | alloc_flags);
1412 if (resp_data) {
1413 kfree(lun_data);
1414 lun_data = resp_data;
1415 length = resp_length;
1416 goto retry;
1417 }
1418 printk(ALLOC_FAILURE_MSG, __func__);
1419 } else
1420 length = get_unaligned_be32(lun_data->scsi_lun);
1421
1422 num_luns = (length / sizeof(struct scsi_lun));
1423
1424 SCSI_LOG_SCAN_BUS(3, sdev_printk (KERN_INFO, sdev,
1425 "scsi scan: REPORT LUN scan\n"));
1426
1427 /*
1428 * Scan the luns in lun_data. The entry at offset 0 is really
1429 * the header, so start at 1 and go up to and including num_luns.
1430 */
1431 for (lunp = &lun_data[1]; lunp <= &lun_data[num_luns]; lunp++) {
1432 lun = scsilun_to_int(lunp);
1433
1434 if (lun > sdev->host->max_lun) {
1435 sdev_printk(KERN_WARNING, sdev,
1436 "lun%llu has a LUN larger than"
1437 " allowed by the host adapter\n", lun);
1438 } else {
1439 int res;
1440
1441 res = scsi_probe_and_add_lun(starget,
1442 lun, NULL, NULL, rescan, NULL);
1443 if (res == SCSI_SCAN_NO_RESPONSE) {
1444 /*
1445 * Got some results, but now none, abort.
1446 */
1447 sdev_printk(KERN_ERR, sdev,
1448 "Unexpected response"
1449 " from lun %llu while scanning, scan"
1450 " aborted\n", (unsigned long long)lun);
1451 break;
1452 }
1453 }
1454 }
1455
1456 out_err:
1457 kfree(lun_data);
1458 out:
1459 if (scsi_device_created(sdev))
1460 /*
1461 * the sdev we used didn't appear in the report luns scan
1462 */
1463 __scsi_remove_device(sdev);
1464 scsi_device_put(sdev);
1465 return ret;
1466 }
1467
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 82a00d7751b3..0a344653487d 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1290,7 +1290,8 @@ static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflag struct scsi_sense_hdr sshdr; struct scsi_device *sdev; struct Scsi_Host *shost = dev_to_shost(&starget->dev); - int ret = 0; + int ret = 0, alloc_flags = + shost->unchecked_isa_dma ? __GFP_DMA : 0; /* * Only support SCSI-3 and up devices if BLIST_NOREPORTLUN is not set. @@ -1327,14 +1328,13 @@ static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflag * value of the now removed max_report_luns parameter. */ length = (511 + 1) * sizeof(struct scsi_lun); -retry: - lun_data = kmalloc(length, GFP_KERNEL | - (sdev->host->unchecked_isa_dma ? __GFP_DMA : 0)); + lun_data = kmalloc(length, GFP_KERNEL | alloc_flags); if (!lun_data) { printk(ALLOC_FAILURE_MSG, __func__); goto out; } +retry: scsi_cmd[0] = REPORT_LUNS; /* @@ -1395,12 +1395,21 @@ static int scsi_report_lun_scan(struct scsi_target *starget, blist_flags_t bflag */ if (get_unaligned_be32(lun_data->scsi_lun) + sizeof(struct scsi_lun) > length) { - length = get_unaligned_be32(lun_data->scsi_lun) + + unsigned int resp_length; + void *resp_data; + + resp_length = get_unaligned_be32(lun_data->scsi_lun) + sizeof(struct scsi_lun); - kfree(lun_data); - goto retry; - } - length = get_unaligned_be32(lun_data->scsi_lun); + resp_data = kmalloc(resp_length, GFP_KERNEL | alloc_flags); + if (resp_data) { + kfree(lun_data); + lun_data = resp_data; + length = resp_length; + goto retry; + } + printk(ALLOC_FAILURE_MSG, __func__); + } else + length = get_unaligned_be32(lun_data->scsi_lun); num_luns = (length / sizeof(struct scsi_lun));
Devices might report a really large REPORT LUN buffer, which will cause kmalloc to bail if it can't allocate enough continuguous pages. However, at that time we already have received a perfectly good response, so we should continue using that buffer to allow us to register at least the LUNs from the 'good' response. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/scsi_scan.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)