* [PATCH v2] fdc: check null block pointer before r/w data transfer
@ 2020-09-22 9:27 P J P
2020-09-22 10:42 ` Li Qiang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: P J P @ 2020-09-22 9:27 UTC (permalink / raw)
To: John Snow
Cc: Ruhr-University, Li Qiang, QEMU Developers, qemu-block,
Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
While transferring data via fdctrl_read/write_data() routines,
check that current drive does not have a null block pointer.
Avoid null pointer dereference.
-> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Ffdc_nullptr1
==1658854==Hint: address points to the zero page.
#0 blk_inc_in_flight block/block-backend.c:1327
#1 blk_prw block/block-backend.c:1299
#2 blk_pwrite block/block-backend.c:1464
#3 fdctrl_write_data hw/block/fdc.c:2418
#4 fdctrl_write hw/block/fdc.c:962
#5 portio_write ioport.c:205
#6 memory_region_write_accessor memory.c:483
#7 access_with_adjusted_size memory.c:544
#8 memory_region_dispatch_write memory.c:1476
Reported-by: Ruhr-University <bugs-syssec@rub.de>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
hw/block/fdc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Update v2: treat NULL blk pointer as an error
-> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06642.html
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 224bac504f..bf968dc66f 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1923,7 +1923,8 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
fd_sector(cur_drv));
return 0;
}
- if (blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
+ if (!cur_drv->blk
+ || blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
BDRV_SECTOR_SIZE)
< 0) {
FLOPPY_DPRINTF("error getting sector %d\n",
@@ -2427,7 +2428,8 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
if (pos == FD_SECTOR_LEN - 1 ||
fdctrl->data_pos == fdctrl->data_len) {
cur_drv = get_cur_drv(fdctrl);
- if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
+ if (!cur_drv->blk
+ || blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
BDRV_SECTOR_SIZE, 0) < 0) {
FLOPPY_DPRINTF("error writing sector %d\n",
fd_sector(cur_drv));
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] fdc: check null block pointer before r/w data transfer
2020-09-22 9:27 [PATCH v2] fdc: check null block pointer before r/w data transfer P J P
@ 2020-09-22 10:42 ` Li Qiang
2021-09-03 12:56 ` Salvatore Bonaccorso
2020-10-01 15:38 ` John Snow
2021-05-18 17:32 ` John Snow
2 siblings, 1 reply; 6+ messages in thread
From: Li Qiang @ 2020-09-22 10:42 UTC (permalink / raw)
To: P J P
Cc: Ruhr-University, John Snow, QEMU Developers, qemu-block,
Prasad J Pandit
P J P <ppandit@redhat.com> 于2020年9月22日周二 下午5:29写道：
>
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While transferring data via fdctrl_read/write_data() routines,
> check that current drive does not have a null block pointer.
> Avoid null pointer dereference.
>
> -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Ffdc_nullptr1
> ==1658854==Hint: address points to the zero page.
> #0 blk_inc_in_flight block/block-backend.c:1327
> #1 blk_prw block/block-backend.c:1299
> #2 blk_pwrite block/block-backend.c:1464
> #3 fdctrl_write_data hw/block/fdc.c:2418
> #4 fdctrl_write hw/block/fdc.c:962
> #5 portio_write ioport.c:205
> #6 memory_region_write_accessor memory.c:483
> #7 access_with_adjusted_size memory.c:544
> #8 memory_region_dispatch_write memory.c:1476
>
> Reported-by: Ruhr-University <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/block/fdc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Update v2: treat NULL blk pointer as an error
> -> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06642.html
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 224bac504f..bf968dc66f 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1923,7 +1923,8 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
> fd_sector(cur_drv));
> return 0;
> }
> - if (blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> + if (!cur_drv->blk
> + || blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> BDRV_SECTOR_SIZE)
> < 0) {
> FLOPPY_DPRINTF("error getting sector %d\n",
> @@ -2427,7 +2428,8 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
> if (pos == FD_SECTOR_LEN - 1 ||
> fdctrl->data_pos == fdctrl->data_len) {
> cur_drv = get_cur_drv(fdctrl);
> - if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> + if (!cur_drv->blk
> + || blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> BDRV_SECTOR_SIZE, 0) < 0) {
> FLOPPY_DPRINTF("error writing sector %d\n",
> fd_sector(cur_drv));
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] fdc: check null block pointer before r/w data transfer
2020-09-22 10:42 ` Li Qiang
@ 2021-09-03 12:56 ` Salvatore Bonaccorso
0 siblings, 0 replies; 6+ messages in thread
From: Salvatore Bonaccorso @ 2021-09-03 12:56 UTC (permalink / raw)
To: Li Qiang
Cc: Prasad J Pandit, qemu-block, QEMU Developers, P J P,
Ruhr-University, John Snow
On Tue, Sep 22, 2020 at 06:42:22PM +0800, Li Qiang wrote:
> P J P <ppandit@redhat.com> 于2020年9月22日周二 下午5:29写道：
> >
> > From: Prasad J Pandit <pjp@fedoraproject.org>
> >
> > While transferring data via fdctrl_read/write_data() routines,
> > check that current drive does not have a null block pointer.
> > Avoid null pointer dereference.
> >
> > -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Ffdc_nullptr1
> > ==1658854==Hint: address points to the zero page.
> > #0 blk_inc_in_flight block/block-backend.c:1327
> > #1 blk_prw block/block-backend.c:1299
> > #2 blk_pwrite block/block-backend.c:1464
> > #3 fdctrl_write_data hw/block/fdc.c:2418
> > #4 fdctrl_write hw/block/fdc.c:962
> > #5 portio_write ioport.c:205
> > #6 memory_region_write_accessor memory.c:483
> > #7 access_with_adjusted_size memory.c:544
> > #8 memory_region_dispatch_write memory.c:1476
> >
> > Reported-by: Ruhr-University <bugs-syssec@rub.de>
> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
Did this one just felt through the cracks, or was it not further
considered?
Regards,
Salvatore
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] fdc: check null block pointer before r/w data transfer
2020-09-22 9:27 [PATCH v2] fdc: check null block pointer before r/w data transfer P J P
2020-09-22 10:42 ` Li Qiang
@ 2020-10-01 15:38 ` John Snow
2021-05-18 17:32 ` John Snow
2 siblings, 0 replies; 6+ messages in thread
From: John Snow @ 2020-10-01 15:38 UTC (permalink / raw)
To: P J P
Cc: Ruhr-University, Li Qiang, QEMU Developers, qemu-block,
Prasad J Pandit
On 9/22/20 5:27 AM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While transferring data via fdctrl_read/write_data() routines,
> check that current drive does not have a null block pointer.
> Avoid null pointer dereference.
>
Will get to these and other IDE issues ASAP.
--js
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] fdc: check null block pointer before r/w data transfer
2020-09-22 9:27 [PATCH v2] fdc: check null block pointer before r/w data transfer P J P
2020-09-22 10:42 ` Li Qiang
2020-10-01 15:38 ` John Snow
@ 2021-05-18 17:32 ` John Snow
2021-05-19 7:27 ` P J P
2 siblings, 1 reply; 6+ messages in thread
From: John Snow @ 2021-05-18 17:32 UTC (permalink / raw)
To: P J P
Cc: Ruhr-University, Li Qiang, QEMU Developers, qemu-block,
Prasad J Pandit
On 9/22/20 5:27 AM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> While transferring data via fdctrl_read/write_data() routines,
> check that current drive does not have a null block pointer.
> Avoid null pointer dereference.
>
> -> https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Ffdc_nullptr1
> ==1658854==Hint: address points to the zero page.
> #0 blk_inc_in_flight block/block-backend.c:1327
> #1 blk_prw block/block-backend.c:1299
> #2 blk_pwrite block/block-backend.c:1464
> #3 fdctrl_write_data hw/block/fdc.c:2418
> #4 fdctrl_write hw/block/fdc.c:962
> #5 portio_write ioport.c:205
> #6 memory_region_write_accessor memory.c:483
> #7 access_with_adjusted_size memory.c:544
> #8 memory_region_dispatch_write memory.c:1476
>
> Reported-by: Ruhr-University <bugs-syssec@rub.de>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
> hw/block/fdc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Update v2: treat NULL blk pointer as an error
> -> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg06642.html
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 224bac504f..bf968dc66f 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1923,7 +1923,8 @@ static uint32_t fdctrl_read_data(FDCtrl *fdctrl)
> fd_sector(cur_drv));
> return 0;
> }
> - if (blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> + if (!cur_drv->blk
> + || blk_pread(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> BDRV_SECTOR_SIZE)
> < 0) {
> FLOPPY_DPRINTF("error getting sector %d\n",
> @@ -2427,7 +2428,8 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value)
> if (pos == FD_SECTOR_LEN - 1 ||
> fdctrl->data_pos == fdctrl->data_len) {
> cur_drv = get_cur_drv(fdctrl);
> - if (blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> + if (!cur_drv->blk
> + || blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
> BDRV_SECTOR_SIZE, 0) < 0) {
> FLOPPY_DPRINTF("error writing sector %d\n",
> fd_sector(cur_drv));
> --
> 2.26.2
>
This patch actually looks good to me. Though again, I assume it can be
rolled into the most recent issue that actually grabbed my attention:
https://gitlab.com/qemu-project/qemu/-/issues/338
And we can credit both reporters (and Alexander) and solve all of these
issues all at once.
I am therefore dropping this patch from my queue. Please let me know if
I am mistaken.
Thanks,
--js
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] fdc: check null block pointer before r/w data transfer
2021-05-18 17:32 ` John Snow
@ 2021-05-19 7:27 ` P J P
0 siblings, 0 replies; 6+ messages in thread
From: P J P @ 2021-05-19 7:27 UTC (permalink / raw)
To: John Snow; +Cc: Ruhr-University, Li Qiang, QEMU Developers, qemu-block
+-- On Tue, 18 May 2021, John Snow wrote --+
| I assume it can be rolled into the most recent issue that actually grabbed
| my attention:
|
| -> https://gitlab.com/qemu-project/qemu/-/issues/338
|
| And we can credit both reporters (and Alexander) and solve all of these issues
| all at once.
|
| I am therefore dropping this patch from my queue. Please let me know if I am
| mistaken.
* It should be okay to collate patches together under above gitlab issue as a
series.
Considering they've individual CVEs assigned, we'll still need to tag them
with CVEs, so that it's easier for downstream users to pick them.
Thank you.
--
- P J P
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-03 12:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-22 9:27 [PATCH v2] fdc: check null block pointer before r/w data transfer P J P
2020-09-22 10:42 ` Li Qiang
2021-09-03 12:56 ` Salvatore Bonaccorso
2020-10-01 15:38 ` John Snow
2021-05-18 17:32 ` John Snow
2021-05-19 7:27 ` P J P
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).