Re: [PATCH] fdc: check null block pointer before blk_pwrite

2021-05-18 Thread John Snow 

On 8/27/20 7:38 AM, P J P wrote:

From: Prasad J Pandit 

While transferring data via fdctrl_write_data(), check that
current drive does not have a null block pointer. Avoid
null pointer dereference.



Hi PJP. I assume this patch actually covers the exact same thing that 
the other if cur_drv->blk patch we have been discussing recently does.


You may want to combine the Reported-By lines for both.

I am dropping this patch from my queue in favor of pursuing our other, 
more recent and active thread, which I am also now tracking via this 
gitlab issue:


https://gitlab.com/qemu-project/qemu/-/issues/338


  -> 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 
Signed-off-by: Prasad J Pandit 
---
  hw/block/fdc.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e9ed3eef45..dedadac68a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2419,7 +2419,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));

Re: [PATCH] fdc: check null block pointer before blk_pwrite

2020-09-18 Thread Li Qiang 
P J P  于2020年8月27日周四 下午7:41写道：
>
> From: Prasad J Pandit 
>
> While transferring data via fdctrl_write_data(), 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 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/block/fdc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index e9ed3eef45..dedadac68a 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2419,7 +2419,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));
> --

Hello Prasad,

There are several pattern to address this NULL check in fdc.c.
I think it is better to consider this as an error. Just like the check
in 'fdctrl_format_sector':

if (cur_drv->blk == NULL ||
blk_pwrite(cur_drv->blk, fd_offset(cur_drv), fdctrl->fifo,
BDRV_SECTOR_SIZE, 0) < 0) {
FLOPPY_DPRINTF("error formatting sector %d\n", fd_sector(cur_drv));
fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00);
} else {

Also there seems exists the same issue in  'fdctrl_read_data'

Thanks,
Li Qiang

> 2.26.2
>
>

Re: [PATCH] fdc: check null block pointer before blk_pwrite

2020-09-15 Thread P J P 
+-- On Thu, 27 Aug 2020, P J P wrote --+
| While transferring data via fdctrl_write_data(), 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 
| Signed-off-by: Prasad J Pandit 
| ---
|  hw/block/fdc.c | 3 ++-
|  1 file changed, 2 insertions(+), 1 deletion(-)
| 
| diff --git a/hw/block/fdc.c b/hw/block/fdc.c
| index e9ed3eef45..dedadac68a 100644
| --- a/hw/block/fdc.c
| +++ b/hw/block/fdc.c
| @@ -2419,7 +2419,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));
| 

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

[PATCH] fdc: check null block pointer before blk_pwrite

2020-08-27 Thread P J P 
From: Prasad J Pandit 

While transferring data via fdctrl_write_data(), 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 
Signed-off-by: Prasad J Pandit 
---
 hw/block/fdc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e9ed3eef45..dedadac68a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2419,7 +2419,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