| From stable+bounces-27047-greg=kroah.com@vger.kernel.org Thu Mar 7 05:21:56 2024 |
| From: Genjian <zhanggenjian@126.com> |
| Date: Thu, 7 Mar 2024 12:14:10 +0800 |
| Subject: loop: Check for overflow while configuring loop |
| To: stable@vger.kernel.org |
| Cc: axboe@kernel.dk, stable@kernel.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, zhanggenjian123@gmail.com, Siddh Raman Pant <code@siddh.me>, syzbot+a8e049cd3abd342936b6@syzkaller.appspotmail.com, Matthew Wilcox <willy@infradead.org>, Christoph Hellwig <hch@lst.de>, Genjian Zhang <zhanggenjian@kylinos.cn> |
| Message-ID: <20240307041411.3792061-8-zhanggenjian@126.com> |
| |
| From: Siddh Raman Pant <code@siddh.me> |
| |
| [ Upstream commit c490a0b5a4f36da3918181a8acdc6991d967c5f3 ] |
| |
| The userspace can configure a loop using an ioctl call, wherein |
| a configuration of type loop_config is passed (see lo_ioctl()'s |
| case on line 1550 of drivers/block/loop.c). This proceeds to call |
| loop_configure() which in turn calls loop_set_status_from_info() |
| (see line 1050 of loop.c), passing &config->info which is of type |
| loop_info64*. This function then sets the appropriate values, like |
| the offset. |
| |
| loop_device has lo_offset of type loff_t (see line 52 of loop.c), |
| which is typdef-chained to long long, whereas loop_info64 has |
| lo_offset of type __u64 (see line 56 of include/uapi/linux/loop.h). |
| |
| The function directly copies offset from info to the device as |
| follows (See line 980 of loop.c): |
| lo->lo_offset = info->lo_offset; |
| |
| This results in an overflow, which triggers a warning in iomap_iter() |
| due to a call to iomap_iter_done() which has: |
| WARN_ON_ONCE(iter->iomap.offset > iter->pos); |
| |
| Thus, check for negative value during loop_set_status_from_info(). |
| |
| Bug report: https://syzkaller.appspot.com/bug?id=c620fe14aac810396d3c3edc9ad73848bf69a29e |
| |
| Reported-and-tested-by: syzbot+a8e049cd3abd342936b6@syzkaller.appspotmail.com |
| Cc: stable@vger.kernel.org |
| Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> |
| Signed-off-by: Siddh Raman Pant <code@siddh.me> |
| Reviewed-by: Christoph Hellwig <hch@lst.de> |
| Link: https://lore.kernel.org/r/20220823160810.181275-1-code@siddh.me |
| Signed-off-by: Jens Axboe <axboe@kernel.dk> |
| Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/block/loop.c | 5 +++++ |
| 1 file changed, 5 insertions(+) |
| |
| --- a/drivers/block/loop.c |
| +++ b/drivers/block/loop.c |
| @@ -1298,6 +1298,11 @@ loop_set_status_from_info(struct loop_de |
| |
| lo->lo_offset = info->lo_offset; |
| lo->lo_sizelimit = info->lo_sizelimit; |
| + |
| + /* loff_t vars have been assigned __u64 */ |
| + if (lo->lo_offset < 0 || lo->lo_sizelimit < 0) |
| + return -EOVERFLOW; |
| + |
| memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE); |
| memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE); |
| lo->lo_file_name[LO_NAME_SIZE-1] = 0; |