Function alloc_disk() returns a NULL pointer if there is no enough memory. In function nbd_init() defined in file drivers/block/nbd.c, when the call to alloc_disk() fails (at line 933), the control flow jumps to label "out", frees allocated memory and returns the error code (stored in variable err). However, because alloc_disk() is called in a loop and the error code (err) is reset (at line 946) in the rest of the loop body, it will return 0 when the call to alloc_disk() fails during the second or after repeat of the loop. In this case, it frees the allocated memory (between lines 1001 and 1006) but notifies the caller that the initialization succeeds. As a result, the caller may continue even when error occurs, the freed memory may be used or freed again, resulting in use-after-free or double free bugs. I think it's better to explicitly assign "-ENOMEM" to err on the true branch of the conditional statement at line 933. In addition, there are two other bugs in the same function. First, when the call to blk_mq_init_queue() (at line 957) fails, err keeps value 0, so it should be explicitly assigned with an error code. Second, function blk_mq_init_queue() returns ERR_PTR pointer rather than a NULL pointer, so I think the check statement at line 958 should be replaced with "if(IS_ERR(disk->queue))". Codes related to these bugs are summarised as follows. nbd_init @@ drivers/block/nbd.c 893 static int __init nbd_init(void) 894 { 895 int err = -ENOMEM; ... 931 for (i = 0; i < nbds_max; i++) { 932 struct gendisk *disk = alloc_disk(1 << part_shift); 933 if (!disk) // (1) add "err = -ENOMEM" here? 934 goto out; 935 nbd_dev[i].disk = disk; ... 946 err = blk_mq_alloc_tag_set(&nbd_dev[i].tag_set); 947 if (err) { 948 put_disk(disk); 949 goto out; 950 } 951 952 /* 953 * The new linux 2.5 block layer implementation requires 954 * every gendisk to have its very own request_queue struct. 955 * These structs are big so we dynamically allocate them. 956 */ 957 disk->queue = blk_mq_init_queue(&nbd_dev[i].tag_set); 958 if (!disk->queue) { // (2) use if(IS_ERR(disk->queue)) ? 959 blk_mq_free_tag_set(&nbd_dev[i].tag_set); 960 put_disk(disk); // (3) add "err = PTR_ERR(disk->queue)" here? 961 goto out; 962 } ... 974 } ... 999 return 0; 1000 out: 1001 while (i--) { 1002 blk_mq_free_tag_set(&nbd_dev[i].tag_set); 1003 blk_cleanup_queue(nbd_dev[i].disk->queue); 1004 put_disk(nbd_dev[i].disk); 1005 } 1006 kfree(nbd_dev); 1007 return err; 1008 } Thanks very much!
The bug has been fixed in linux-v4.10. So I closed it.