Bug 188441

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!