[bug report] block: fix leaking page ref on truncated direct io

[bug report] block: fix leaking page ref on truncated direct io

[Dan Carpenter]

Hello Keith Busch,

The patch 7b1ccdf617ca: "block: fix leaking page ref on truncated
direct io" from Jul 5, 2022, leads to the following Smatch static
checker warning:

	block/bio.c:1254 __bio_iov_iter_get_pages()
	error: uninitialized symbol 'i'.

block/bio.c
    1195 static int __bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter)
    1196 {
    1197         unsigned short nr_pages = bio->bi_max_vecs - bio->bi_vcnt;
    1198         unsigned short entries_left = bio->bi_max_vecs - bio->bi_vcnt;
    1199         struct bio_vec *bv = bio->bi_io_vec + bio->bi_vcnt;
    1200         struct page **pages = (struct page **)bv;
    1201         ssize_t size, left;
    1202         unsigned len, i;
    1203         size_t offset, trim;
    1204         int ret = 0;
    1205 
    1206         /*
    1207          * Move page array up in the allocated memory for the bio vecs as far as
    1208          * possible so that we can start filling biovecs from the beginning
    1209          * without overwriting the temporary page array.
    1210          */
    1211         BUILD_BUG_ON(PAGE_PTRS_PER_BVEC < 2);
    1212         pages += entries_left * (PAGE_PTRS_PER_BVEC - 1);
    1213 
    1214         /*
    1215          * Each segment in the iov is required to be a block size multiple.
    1216          * However, we may not be able to get the entire segment if it spans
    1217          * more pages than bi_max_vecs allows, so we have to ALIGN_DOWN the
    1218          * result to ensure the bio's total size is correct. The remainder of
    1219          * the iov data will be picked up in the next bio iteration.
    1220          */
    1221         size = iov_iter_get_pages2(iter, pages, UINT_MAX - bio->bi_iter.bi_size,
    1222                                   nr_pages, &offset);
    1223         if (unlikely(size <= 0))
    1224                 return size ? size : -EFAULT;
    1225 
    1226         nr_pages = DIV_ROUND_UP(offset + size, PAGE_SIZE);
    1227 
    1228         trim = size & (bdev_logical_block_size(bio->bi_bdev) - 1);
    1229         iov_iter_revert(iter, trim);
    1230 
    1231         size -= trim;
    1232         if (unlikely(!size)) {
    1233                 ret = -EFAULT;
    1234                 goto out;

"i" is uninitialized on this path.  (You probably have already fixed
this and recieved a million other static checker notifications).

    1235         }
    1236 
    1237         for (left = size, i = 0; left > 0; left -= len, i++) {
    1238                 struct page *page = pages[i];
    1239 
    1240                 len = min_t(size_t, PAGE_SIZE - offset, left);
    1241                 if (bio_op(bio) == REQ_OP_ZONE_APPEND) {
    1242                         ret = bio_iov_add_zone_append_page(bio, page, len,
    1243                                         offset);
    1244                         if (ret)
    1245                                 break;
    1246                 } else
    1247                         bio_iov_add_page(bio, page, len, offset);
    1248 
    1249                 offset = 0;
    1250         }
    1251 
    1252         iov_iter_revert(iter, left);
    1253 out:
--> 1254         while (i < nr_pages)
    1255                 put_page(pages[i++]);
    1256 
    1257         return ret;
    1258 }

regards,
dan carpenter

Re: [bug report] block: fix leaking page ref on truncated direct io

[Keith Busch]

On Tue, Jul 12, 2022 at 11:52:53AM +0300, Dan Carpenter wrote:
> Hello Keith Busch,
> 
> The patch 7b1ccdf617ca: "block: fix leaking page ref on truncated
> direct io" from Jul 5, 2022, leads to the following Smatch static
> checker warning:
> 
> 	block/bio.c:1254 __bio_iov_iter_get_pages()
> 	error: uninitialized symbol 'i'.

Yeah, this one was not ready to be merged into any staging trees. One of the
build-bots even flagged this patch hours after it was initially posted.

I've a v2 posted with the appropriate correction, and I'll sort it out with the
maintainers on next steps.