scsi: target: pscsi: Avoid OOM in pscsi_map_sg()

pscsi_map_sg() uses the variable nr_pages as a hint for bio_kmalloc() how
many vector elements to allocate. If nr_pages is < BIO_MAX_PAGES, it will
be reset to 0 after successful allocation of the bio.

If bio_add_pc_page() fails later for whatever reason, pscsi_map_sg() tries
to allocate another bio, passing nr_vecs = 0. This causes bio_add_pc_page()
to fail immediately in the next call. pci_map_sg() continues to allocate
zero-length bios until memory is exhausted and the kernel crashes with
OOM. This can be easily observed by exporting a SATA DVD drive via pscsi.
The target crashes as soon as the client tries to access the DVD LUN. In
the case I analyzed, bio_add_pc_page() would fail because the DVD device's
max_sectors_kb (128) was exceeded.

Avoid this by simply not resetting nr_pages to 0 after allocating the
bio. This way, the client receives an I/O error when it tries to send
requests exceeding the devices max_sectors_kb, and eventually gets it
right. The client must still limit max_sectors_kb e.g. by an udev rule if
(like in my case) the driver doesn't report valid block limits, otherwise
it encounters I/O errors.

Link: https://lore.kernel.org/r/20210323212431.15306-1-mwilck@suse.com
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 3cbc074992bc86af1606acf481c2ac60f49bba58..6fa9fddc2e1e097f9a79333b3149ca79828dcb7b 100644 (file)
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -882,7 +882,6 @@ pscsi_map_sg(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
                        if (!bio) {
 new_bio:
                                nr_vecs = bio_max_segs(nr_pages);
-                               nr_pages -= nr_vecs;
                                /*
                                 * Calls bio_kmalloc() and sets bio->bi_end_io()
                                 */