Re: [PATCH] xen-scsifront: Add a missing call to kfree

[Dan Carpenter]

On Mon, Nov 21, 2016 at 07:01:36AM +0100, Juergen Gross wrote:
> On 19/11/16 19:22, Quentin Lambert wrote:
> > Most error branches following the call to kmalloc contain
> > a call to kfree. This patch add these calls where they are
> > missing.
> > 
> > This issue was found with Hector.
> > 
> > Signed-off-by: Quentin Lambert 
> 
> Nice catch. I think this will need some more work, I'll do a
> follow-on patch.

Yeah.  It's weird how we free it on the success path and all the failure
paths except one.  But it looks so deliberate.  What's going on with
that?

Could you send your follow on patch as a reply to the thread?

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

scsi: use-after-free in bio_copy_from_iter

[Dmitry Vyukov]

Hello,

The following program triggers use-after-free in bio_copy_from_iter:
https://gist.githubusercontent.com/dvyukov/80cd94b4e4c288f16ee4c787d404118b/raw/10536069562444da51b758bb39655b514ff93b45/gistfile1.txt


==
BUG: KASAN: use-after-free in copy_from_iter+0xf30/0x15e0 at addr
880062c6e02a
Read of size 4096 by task a.out/8529
page:ea00018b1b80 count:2 mapcount:0 mapping:88006c80e9d0 index:0x1695
flags: 0x5fffc000864(referenced|lru|active|private)
page dumped because: kasan: bad access detected
page->mem_cgroup:88003ebcea00
CPU: 1 PID: 8529 Comm: a.out Not tainted 4.9.0-rc6+ #55
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 880039ca6770 834c3bb9 0001 110007394c81
 ed0007394c79 41b58ab3 89575c30 834c38cb
   0001 
Call Trace:
 [< inline >] __dump_stack lib/dump_stack.c:15
 [] dump_stack+0x2ee/0x3f5 lib/dump_stack.c:51
 [< inline >] print_address_description mm/kasan/report.c:211
 [< inline >] kasan_report_error mm/kasan/report.c:285
 [] kasan_report+0x418/0x440 mm/kasan/report.c:305
 [< inline >] check_memory_region_inline mm/kasan/kasan.c:308
 [] check_memory_region+0x139/0x190 mm/kasan/kasan.c:315
 [] memcpy+0x23/0x50 mm/kasan/kasan.c:350
 [] copy_from_iter+0xf30/0x15e0 lib/iov_iter.c:559
 [] copy_page_from_iter+0x474/0x860 lib/iov_iter.c:614
 [< inline >] bio_copy_from_iter block/bio.c:1025
 [] bio_copy_user_iov+0xb50/0xf10 block/bio.c:1226
 [< inline >] __blk_rq_map_user_iov block/blk-map.c:56
 [] blk_rq_map_user_iov+0x295/0x930 block/blk-map.c:130
 [] blk_rq_map_user+0x139/0x1e0 block/blk-map.c:159
 [< inline >] sg_start_req drivers/scsi/sg.c:1757
 [] sg_common_write.isra.20+0x12da/0x1b20
drivers/scsi/sg.c:772
 [] sg_write+0x78a/0xda0 drivers/scsi/sg.c:675
 [] __vfs_write+0x65e/0x830 fs/read_write.c:510
 [] __kernel_write+0xec/0x340 fs/read_write.c:532
 [] write_pipe_buf+0x19c/0x260 fs/splice.c:814
 [< inline >] splice_from_pipe_feed fs/splice.c:519
 [] __splice_from_pipe+0x31f/0x750 fs/splice.c:643
 [] splice_from_pipe+0x1dc/0x300 fs/splice.c:678
 [] default_file_splice_write+0x45/0x90 fs/splice.c:826
 [< inline >] do_splice_from fs/splice.c:868
 [] direct_splice_actor+0x12a/0x190 fs/splice.c:1035
 [] splice_direct_to_actor+0x2c6/0x840 fs/splice.c:990
 [] do_splice_direct+0x2ce/0x470 fs/splice.c:1078
 [] do_sendfile+0x73f/0x1060 fs/read_write.c:1401
 [< inline >] SYSC_sendfile64 fs/read_write.c:1456
 [] SyS_sendfile64+0xee/0x1a0 fs/read_write.c:1448
 [] entry_SYSCALL_64_fastpath+0x23/0xc6
arch/x86/entry/entry_64.S:209
Memory state around the buggy address:
 880062c6ef00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 880062c6ef80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>880062c6f000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
   ^
 880062c6f080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 880062c6f100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==
Disabling lock debugging due to kernel taint
BUG: unable to handle kernel paging request at 880062c6f000
IP: [] __memcpy+0x12/0x20 arch/x86/lib/memcpy_64.S:37
PGD c53d067 [  494.351750] PUD c540067
PMD 7fdea067 [  494.351750] PTE 800062c6f060

Oops:  [#1] SMP DEBUG_PAGEALLOC KASAN
Modules linked in:
CPU: 1 PID: 8529 Comm: a.out Tainted: GB   4.9.0-rc6+ #55
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88003c54e3c0 task.stack: 880039ca
RIP: 0010:[]  []
__memcpy+0x12/0x20 arch/x86/lib/memcpy_64.S:37
RSP: 0018:880039ca6838  EFLAGS: 00010246
RAX: 880031b99000 RBX: 1000 RCX: 0006
RDX:  RSI: 880062c6effa RDI: 880031b99fd0
RBP: 880039ca6858 R08:  R09: 
R10: 0200 R11: ed00063733ff R12: 880031b99000
R13: 880062c6e02a R14: 880039ca6f70 R15: 880039ca6d18
FS:  7f7a78013700() GS:88003ed0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 880062c6f000 CR3: 6411f000 CR4: 06e0
Stack:
 819f0a65 880039ca71a0 1000 2000
 880039ca6d40 83525d10 41b58ab3 89576240
 0001  41b58ab3 894d07b0
Call Trace:
 [] copy_from_iter+0xf30/0x15e0 lib/iov_iter.c:559
 [] copy_page_from_iter+0x474/0x860 lib/iov_iter.c:614
 [< inline >] bio_copy_from_iter block/bio.c:1025
 [] bio_copy_user_iov+0xb50/0xf10 block/bio.c:1226
 [< inline >] __blk_rq_map_user_iov block/blk-map.c:56
 [] blk_rq_map_user_iov+0x295/0x930 block/blk-map.c:130
 [] blk_rq_map_user+0x139/0x1e0 

Re: SG does not ignore dxferp (direct io + mmap)

[Laurence Oberman]

- Original Message -
> From: "Ewan Milne" 
> To: "Johannes Thumshirn" 
> Cc: "Laurence Oberman" , "Eyal Ben David" 
> , dgilb...@interlog.com,
> linux-scsi@vger.kernel.org
> Sent: Friday, November 25, 2016 12:56:16 PM
> Subject: Re: SG does not ignore dxferp (direct io + mmap)
> 
> >> ---
> >> 
> >> In other words, this commit made the bad behavior go away in 4.8.
> >> Need to look at this in more detail, it doesn't appear as if this patch
> >> was intended to fix such a problem.
> >> 
> >> -Ewan
> >
> >Are you sure it did? I can repropduce copy_to_user() errors with 4.8 as
> >well.
> >Using the very same reproducer. On 4.8 it's just harder to trigger and
> >doesn't trigger on AHCI as fas as I can telli (maybe I just haven't hit
> >it hard enough). I can trigger it on QEMUs SCSI CDROM emulation and hpsa
> >though. I cannot however trigger this with a minimal config inside an
> >initrd.
> 
> It did for Eyal's supplied test case on my machine, but that was not an
> exhaustive test, and I am a little suspicious that the behavior change was
> due to a side-effect of the patch rather than actually fixing something.
> 
> I think what we need to understand is what caused the regression in the
> first place, I probably should have been bisecting the original failure
> rather than trying to find where it started working.
> 
> I was running against an internal (physical) drive.
> 
> -Ewan
> 

My 10 loop was against an HPSA target and passed all tests.
Again, all I did was patch the 4.7.9 with the 2 line changes, the rest of the 
patch was line breaks.

I guess we need to understand when it first broke and what caused that, versus 
what seems to correct it.
Thanks
Laurence
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Ewan Milne]

>> ---
>> 
>> In other words, this commit made the bad behavior go away in 4.8.
>> Need to look at this in more detail, it doesn't appear as if this patch
>> was intended to fix such a problem.
>> 
>> -Ewan
>
>Are you sure it did? I can repropduce copy_to_user() errors with 4.8 as well.
>Using the very same reproducer. On 4.8 it's just harder to trigger and
>doesn't trigger on AHCI as fas as I can telli (maybe I just haven't hit
>it hard enough). I can trigger it on QEMUs SCSI CDROM emulation and hpsa
>though. I cannot however trigger this with a minimal config inside an initrd.

It did for Eyal's supplied test case on my machine, but that was not an
exhaustive test, and I am a little suspicious that the behavior change was
due to a side-effect of the patch rather than actually fixing something.

I think what we need to understand is what caused the regression in the
first place, I probably should have been bisecting the original failure
rather than trying to find where it started working.

I was running against an internal (physical) drive.

-Ewan
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: fix oops/BUG in lpfc_sli_ringtxcmpl_put()

[Martin K. Petersen]

> "Mauricio" == Mauricio Faria de Oliveira  
> writes:

Mauricio> The BUG_ON() recently introduced in lpfc_sli_ringtxcmpl_put()
Mauricio> is hit in the lpfc_els_abort() > lpfc_sli_issue_abort_iotag()
Mauricio> > lpfc_sli_abort_iotag_issue() function path [similar names],
Mauricio> due to 'piocb->vport == NULL':

Applied to 4.9/scsi-fixes.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] scsi: libfc: Remove an unneeded condition

[Martin K. Petersen]

> "Dan" == Dan Carpenter  writes:

Dan> We verified that resp_code is FC_SPP_RESP_ACK earlier so we don't
Dan> need to check again here.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Laurence Oberman]

- Original Message -
> From: "Eyal Ben David" 
> To: "Johannes Thumshirn" 
> Cc: "Ewan D. Milne" , "Laurence Oberman" 
> , dgilb...@interlog.com,
> linux-scsi@vger.kernel.org
> Sent: Friday, November 25, 2016 7:36:34 AM
> Subject: Re: SG does not ignore dxferp (direct io + mmap)
> 
> On Fri, Nov 25, 2016 at 1:53 PM, Johannes Thumshirn 
> wrote:
> > On Fri, Nov 25, 2016 at 01:20:34PM +0200, Eyal Ben David wrote:
> >> Note that sg_mmap_read does not parse the SCSI sense, so the script
> >> might fail for other reasons (some SCSI error) and think its a zero
> >> byte corruption.
> >
> > But SCSI generic checks for errors and returns -EINVAL on CHECK_CONDITION
> > or
> > DRIVER_SENSE (and sets SG_INFO_CHECK in hdr.info).
> >
> Ah OK. We use async write/read instead of ioctl and forgot that ioctl
> checks the read.
> 
> > Anyways, can you test the patch Ewan found on one of your kernel's that are
> > known to fail?
> 
> All the examples I gave before were on physical hosts and storage at a
> testing lab.
> That would be difficult to do. Sorry.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

I applied just the patch myself and Ewan isolated to the 4.7.9 kernel and the 
issue is resolved for me.
I have 10 loops of the test with no issue.

I repeated the test flushing cache each time as well to make sure

for i in `seq 1 10`; do ./sg_mmap_read -d /dev/sg1 -l 0 -m -b | hexdump | 
grep 6300; done

for i in `seq 1 10`; do ./sg_mmap_read -d /dev/sg1 -l 0 -m -b | hexdump | 
grep 6300; echo 3 > /proc/sys/vm/drop_caches;done

Johannes, you are reproducing another race in your test I think.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] SRP transport, scsi-mq: Wait for .queue_rq() if necessary

[Martin K. Petersen]

> "Martin" == Martin K Petersen  writes:

Hi Bart,

Martin> Applied to 4.10/scsi-queue.

2/2 needs a rebase and I'm not going to do another one this late in the
cycle. Please resend this patch once we hit 4.10 rc1.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] SRP transport, scsi-mq: Wait for .queue_rq() if necessary

[Martin K. Petersen]

> "Bart" == Bart Van Assche  writes:

Bart> The SRP transport code must wait until ongoing .queuecommand() /
Bart> .queue_rq() callback function invocations have finished before
Bart> reconnecting at the transport layer level and also before invoking
Bart> .terminate_rport_io(). This is already the case for the single
Bart> queue path but not yet for the scsi-mq path. This patch series
Bart> realizes the proper serialization for the scsi-mq path. Compared
Bart> to last time these patches were posted, only the patch
Bart> descriptions and one comment have been changed.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ufs: Add missing UFS_MASK macro definition

[Martin K. Petersen]

> "Zhangfei" == Zhangfei Gao  writes:

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/11] hisi_sas: some fixes, improvements, and new features

[Martin K. Petersen]

> "John" == John Garry  writes:

John> I think that these 2 outstanding issues have been
John> addressed. Please let me know if there is anything else.

Series applied to 4.10/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

ata mapping mvsas driver

[Jelle de Jong]

Hello everybody,

I am trying to find the mapping for ata22.00. due to some repeated issues:

# 3.16.36-1+deb8u2~bpo70+1 crazing down
http://paste.debian.net/hidden/54c12fae/

# smart status 16 disks WDC 1TB RE SATA
http://paste.debian.net/hidden/8bfe8d2f/

"[473315.445703] ata22.00: exception Emask 0x0 SAct 0x7fe7 SErr 0x0 
action 0x6 frozen"


lsscsi --verbose doesn’t provide me with the ata details?
http://paste.debian.net/hidden/430599a0/

root@shirley:~# lsscsi --verbose
[0:0:0:0]diskATA  WDC WD1003FBYX-0 1V01  /dev/sdc
  dir: /sys/bus/scsi/devices/0:0:0:0 
[/sys/devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:0/end_device-0:0:0/target0:0:0/0:0:0:0]


I can not find any useful unique_id with the mvsas driver...
find /sys/devices/pci\:00/\:00\:1c.0/\:08\:00.0/ -iname 
unique_id


grep ata22 /var/log/messages indicates it is really mapped somewhere...

root@shirley:~# readlink /sys/block/sdm
../devices/pci:00/:00:1c.0/:08:00.0/host0/port-0:0/expander-0:0/port-0:0:10/end_device-0:0:10/target0:0:10/0:0:10:0/block/sdm

Best I came up with with no results:

root@shirley:~# find 
/sys/devices/pci\:00/\:00\:1c.0/\:08\:00.0/host0/port-0\:0/expander-0\:0/port-0\:0\:10/end_device-0\:0\:10/target0\:0\:10/0\:0\:10\:0/ 
-type f  -exec grep -i ata {} \;


Linux shirley 3.16.0-0.bpo.4-amd64 #1 SMP Debian 
3.16.36-1+deb8u2~bpo70+1 (2016-10-19) x86_64 GNU/Linux

root@shirley:~# lsmod mvsas
Usage: lsmod

root@shirley:~# modinfo mvsas
filename: 
/lib/modules/3.16.0-0.bpo.4-amd64/kernel/drivers/scsi/mvsas/mvsas.ko

license:GPL
version:0.8.16

Thank you in advance!

Kind regards,

Jelle de Jong
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Eyal Ben David]

On Fri, Nov 25, 2016 at 1:53 PM, Johannes Thumshirn  wrote:
> On Fri, Nov 25, 2016 at 01:20:34PM +0200, Eyal Ben David wrote:
>> Note that sg_mmap_read does not parse the SCSI sense, so the script
>> might fail for other reasons (some SCSI error) and think its a zero
>> byte corruption.
>
> But SCSI generic checks for errors and returns -EINVAL on CHECK_CONDITION or
> DRIVER_SENSE (and sets SG_INFO_CHECK in hdr.info).
>
Ah OK. We use async write/read instead of ioctl and forgot that ioctl
checks the read.

> Anyways, can you test the patch Ewan found on one of your kernel's that are
> known to fail?

All the examples I gave before were on physical hosts and storage at a
testing lab.
That would be difficult to do. Sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Johannes Thumshirn]

On Fri, Nov 25, 2016 at 12:53:17PM +0100, Johannes Thumshirn wrote:
> On Fri, Nov 25, 2016 at 01:20:34PM +0200, Eyal Ben David wrote:
> > Note that sg_mmap_read does not parse the SCSI sense, so the script
> > might fail for other reasons (some SCSI error) and think its a zero
> > byte corruption.
> 
> But SCSI generic checks for errors and returns -EINVAL on CHECK_CONDITION or
> DRIVER_SENSE (and sets SG_INFO_CHECK in hdr.info).
> 
> And:
> VM:~ # ./test.sh 
> FAIL on run 2
> Expect:
> 000 8240 3d1f 8800  0002   
> 020     6e9d 57ac  
> 040        
> *
> 100
> Fail:
> 000        
> *
> 100
> VM:~ # uname -r
> 4.8.9-60-default+
> VM:~ # 
> 
> 
> Anyways, can you test the patch Ewan found on one of your kernel's that are
> known to fail?

Hannes found another interesting aspect, if one sets the allow_dio module
parameter to one and sets the SG_FLAG_DIRECT_IO in the header, it works
reliably.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] scsi: aic94xx: Add a missing call to kfree

[Quentin Lambert]

Most error branches following the call to kzalloc contain
a call to kfree. This patch add these calls where they are
missing and set the relevant pointers to NULL.

This issue was found with Hector.

Signed-off-by: Quentin Lambert 

---
 v2: set the point to NULL after having freed it
 drivers/scsi/aic94xx/aic94xx_hwi.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/drivers/scsi/aic94xx/aic94xx_hwi.c
+++ b/drivers/scsi/aic94xx/aic94xx_hwi.c
@@ -228,8 +228,11 @@ static int asd_init_scbs(struct asd_ha_s
bitmap_bytes = (asd_ha->seq.tc_index_bitmap_bits+7)/8;
bitmap_bytes = BITS_TO_LONGS(bitmap_bytes*8)*sizeof(unsigned long);
asd_ha->seq.tc_index_bitmap = kzalloc(bitmap_bytes, GFP_KERNEL);
-   if (!asd_ha->seq.tc_index_bitmap)
+   if (!asd_ha->seq.tc_index_bitmap) {
+   kfree(asd_ha->seq.tc_index_array);
+   asd_ha->seq.tc_index_array = NULL;
return -ENOMEM;
+   }
 
spin_lock_init(>tc_index_lock);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Johannes Thumshirn]

On Fri, Nov 25, 2016 at 01:20:34PM +0200, Eyal Ben David wrote:
> Note that sg_mmap_read does not parse the SCSI sense, so the script
> might fail for other reasons (some SCSI error) and think its a zero
> byte corruption.

But SCSI generic checks for errors and returns -EINVAL on CHECK_CONDITION or
DRIVER_SENSE (and sets SG_INFO_CHECK in hdr.info).

And:
VM:~ # ./test.sh 
FAIL on run 2
Expect:
000 8240 3d1f 8800  0002   
020     6e9d 57ac  
040        
*
100
Fail:
000        
*
100
VM:~ # uname -r
4.8.9-60-default+
VM:~ # 


Anyways, can you test the patch Ewan found on one of your kernel's that are
known to fail?

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/11] hisi_sas: some fixes, improvements, and new features

[John Garry]

On 22/11/2016 22:01, Martin K. Petersen wrote:

"John" == John Garry  writes:


John,

John> Are you happy with this patchset now that I've got an external
John> review?

Zhangfei Geo asked a question about patch 1/11 that has yet to be
answered. Patch 5/11 is still unreviewed.



Hi Martin,

I think that these 2 outstanding issues have been addressed. Please let 
me know if there is anything else.


Thanks alot,
John

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 189061] New: Function snic_probe() does not set set code when the call to mempool_create_slab_pool() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=189061

Bug ID: 189061
   Summary: Function snic_probe() does not set set code when the
call to mempool_create_slab_pool() fails
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

In the function snic_probe() defined in file drivers/scsi/snic/snic_main.c,
when the call to mempool_create_slab_pool() (at line 589) returns a NULL
pointer, the control flow jumps to label "err_free_res", and returns variable
ret. Because variable ret is checked at line 714, the value of ret must be 0
here. As a result, function snic_probe() returns 0 (indicates success) even if
the call to mempool_create_slab_pool() fails.
There are other 2 similar bugs when the call to mempool_create_slab_pool() fail
at lines 599 and 609. Though these errors may occur rarely, I think it may be
better to set correct error codes (e.g. -ENOMEM) on failures. Codes related to
these bugs are summarised as follows.

snic_probe @@ drivers/scsi/snic/snic_main.c
 360 static int
 361 snic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 362 {
 ...
 368 int ret, i;
 ...
 561 ret = snic_alloc_vnic_res(snic);
 562 if (ret) {
 563 SNIC_HOST_ERR(shost,
 564   "Failed to alloc vNIC resources aborting. %d\n",
 565   ret);
 566 
 567 goto err_clear_intr;
 568 }
 ...
 // Insert "ret = -ENOMEM;" ?
 589 pool = mempool_create_slab_pool(2,
 590 snic_glob->req_cache[SNIC_REQ_CACHE_DFLT_SGL]);
 591 if (!pool) {
 592 SNIC_HOST_ERR(shost, "dflt sgl pool creation failed\n");
 593 
 // Bug (1): the value of ret is 0
 594 goto err_free_res;
 595 }
 596 
 597 snic->req_pool[SNIC_REQ_CACHE_DFLT_SGL] = pool;
 598 
 599 pool = mempool_create_slab_pool(2,
 600 snic_glob->req_cache[SNIC_REQ_CACHE_MAX_SGL]);
 601 if (!pool) {
 602 SNIC_HOST_ERR(shost, "max sgl pool creation failed\n");
 603 
 // Bug (2): the value of ret is 0
 604 goto err_free_dflt_sgl_pool;
 605 }
 606 
 607 snic->req_pool[SNIC_REQ_CACHE_MAX_SGL] = pool;
 608 
 609 pool = mempool_create_slab_pool(2,
 610 snic_glob->req_cache[SNIC_REQ_TM_CACHE]);
 611 if (!pool) {
 612 SNIC_HOST_ERR(shost, "snic tmreq info pool creation failed.\n");
 613 
 // Bug (3): the value of ret is 0
 614 goto err_free_max_sgl_pool;
 615 }
 ...
 701 return 0;
 ...
 733 err_free_max_sgl_pool:
 734 mempool_destroy(snic->req_pool[SNIC_REQ_CACHE_MAX_SGL]);
 735 
 736 err_free_dflt_sgl_pool:
 737 mempool_destroy(snic->req_pool[SNIC_REQ_CACHE_DFLT_SGL]);
 738 
 739 err_free_res:
 740 snic_free_vnic_res(snic);
 741 
 742 err_clear_intr:
 743 snic_clear_intr_mode(snic);
 744 
 ...
 772 return ret;
 773 } /* end of snic_probe */

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 189051] New: Function fnic_probe() does not set set code when the call to mempool_create_slab_pool() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=189051

Bug ID: 189051
   Summary: Function fnic_probe() does not set set code when the
call to mempool_create_slab_pool() fails
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

In the function fnic_probe() defined in file drivers/scsi/fnic/fnic_main.c,
when the call to mempool_create_slab_pool() (at line 738) returns a NULL
pointer, the control flow jumps to label "err_out_free_resources", and returns
variable err. Because variable err is checked at line 714, the value of err
must be 0 here. As a result, function fnic_probe() returns 0 (indicates
success) even if the call to mempool_create_slab_pool() fails.
There are other 2 similar bugs when the call to mempool_create_slab_pool() fail
at lines 742 and 747. Though these errors may occur rarely, I think it may be
better to set correct error codes (e.g. -ENOMEM) on failures. Codes related to
these bugs are summarised as follows.

fnic_probe @@ drivers/scsi/fnic/fnic_main.c
 541 static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id
*ent)
 542 {
 ...
 547 int err;
 ...
 713 err = fnic_alloc_vnic_resources(fnic);
 714 if (err) {
 715 shost_printk(KERN_ERR, fnic->lport->host,
 716  "Failed to alloc vNIC resources, "
 717  "aborting.\n");
 718 goto err_out_clear_intr;
 719 }
 ...
 // Insert "err = -ENOMEM;" ?
 738 fnic->io_req_pool = mempool_create_slab_pool(2, fnic_io_req_cache);
 739 if (!fnic->io_req_pool)
 // Bug (1): the value of err is 0
 740 goto err_out_free_resources;
 741 
 742 pool = mempool_create_slab_pool(2,
fnic_sgl_cache[FNIC_SGL_CACHE_DFLT]);
 743 if (!pool)
 // Bug (2): the value of err is 0
 744 goto err_out_free_ioreq_pool;
 745 fnic->io_sgl_pool[FNIC_SGL_CACHE_DFLT] = pool;
 746 
 747 pool = mempool_create_slab_pool(2,
fnic_sgl_cache[FNIC_SGL_CACHE_MAX]);
 748 if (!pool)
 // Bug (3): the value of err is 0
 749 goto err_out_free_dflt_pool;
 ...
 901 return 0;
 ...
 914 err_out_free_dflt_pool:
 915 mempool_destroy(fnic->io_sgl_pool[FNIC_SGL_CACHE_DFLT]);
 916 err_out_free_ioreq_pool:
 917 mempool_destroy(fnic->io_req_pool);
 918 err_out_free_resources:
 919 fnic_free_vnic_resources(fnic);
 920 err_out_clear_intr:
 921 fnic_clear_intr_mode(fnic);
 922 err_out_dev_close:
 923 vnic_dev_close(fnic->vdev);
 924 err_out_vnic_unregister:
 925 vnic_dev_unregister(fnic->vdev);
 926 err_out_iounmap:
 927 fnic_iounmap(fnic);
 928 err_out_release_regions:
 929 pci_release_regions(pdev);
 930 err_out_disable_device:
 931 pci_disable_device(pdev);
 932 err_out_free_hba:
 933 fnic_stats_debugfs_remove(fnic);
 934 scsi_host_put(lp->host);
 935 err_out:
 936 return err;
 937 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Eyal Ben David]

Note that sg_mmap_read does not parse the SCSI sense, so the script
might fail for other reasons (some SCSI error) and think its a zero
byte corruption.


If you think an improved version could help (compare results within
the program + parse senses) I can help.


On Fri, Nov 25, 2016 at 10:07 AM, Johannes Thumshirn  wrote:
> On Wed, Nov 23, 2016 at 03:22:04PM -0500, Ewan Milne wrote:
>
> [...]
>
>> ---
>>
>> In other words, this commit made the bad behavior go away in 4.8.
>> Need to look at this in more detail, it doesn't appear as if this patch
>> was intended to fix such a problem.
>>
>> -Ewan
>
> Are you sure it did? I can repropduce copy_to_user() errors with 4.8 as well.
> Using the very same reproducer. On 4.8 it's just harder to trigger and
> doesn't trigger on AHCI as fas as I can telli (maybe I just haven't hit
> it hard enough). I can trigger it on QEMUs SCSI CDROM emulation and hpsa
> though. I cannot however trigger this with a minimal config inside an initrd.
>
>
> Here's my test:
>
> --- 8< ---
> #!/bin/sh
>
> cleanup()
> {
> rm -f expect.txt
> rm -f test.txt
> }
>
> trap cleanup EXIT INT
>
> ./sg_mmap_read -d /dev/sg0 -l 0 | od -x > expect.txt
>
> for i in $(seq 1 5000); do
> ./sg_mmap_read -d /dev/sg0 -l 0 | od -x > test.txt
> diff expect.txt test.txt > /dev/null
> if [ $? -ne 0 ]; then
> echo "FAIL"
> exit 1
> fi
> done
>
> echo "PASS"
>
> --- >8 ---
>
> I added some trace_printk()s to sg.c and try to hunt it down. It'll be good to
> know if anyone of you can reproduce it with the above script as well.
>
> Byte,
> Johannes
> --
> Johannes Thumshirn  Storage
> jthumsh...@suse.de+49 911 74053 689
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
> Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188951] New: Function beiscsi_create_eqs() may return improper value when the call to pci_alloc_consistent() fails, which may result in use-after-free

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188951

Bug ID: 188951
   Summary: Function beiscsi_create_eqs() may return improper
value when the call to pci_alloc_consistent() fails,
which may result in use-after-free
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

Function pci_alloc_consistent() returns a NULL pointer if there is no enough
memory. In function beiscsi_create_eqs() defined in file
drivers/scsi/be2iscsi/be_main.c, function pci_alloc_consistent() is called and
its return value is checked against NULL (at line 3052). If the return value is
NULL, the control flow will jump to label "create_cq_error", frees allocated
memory and returns variable ret. Because after the first execution of the loop
the value of ret must be 0 (see the check statement of ret at line 3067), the
return value will be 0 (indicates success) if pci_alloc_consistent() fails
during the second or after repeats of the loop body. In this case, the freed
memory may be used or freed again in the callers of beiscsi_create_eqs(). I
think it is better to assign "-ENOMEM" when the call pci_alloc_consistent()
fails. Codes and comments related to this bug are summarised as follows.

beiscsi_create_eqs @@ drivers/scsi/be2iscsi/be_main.c
3028 static int beiscsi_create_eqs(struct beiscsi_hba *phba,
3029  struct hwi_context_memory *phwi_context)
3030 {
3031 int ret = -ENOMEM, eq_for_mcc;
 ...
3045 for (i = 0; i < (phba->num_cpus + eq_for_mcc); i++) {
3046 eq = _context->be_eq[i].q;
3047 mem = >dma_mem;
3048 phwi_context->be_eq[i].phba = phba;
3049 eq_vaddress = pci_alloc_consistent(phba->pcidev,
3050num_eq_pages * PAGE_SIZE,
3051);
3052 if (!eq_vaddress)
 // ret may takes value 0. Add "ret = -ENOMEM" here?
3053 goto create_eq_error;

3065 ret = beiscsi_cmd_eq_create(>ctrl, eq,
3066 phwi_context->cur_eqd);
3067 if (ret) {
3068 beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
3069 "BM_%d : beiscsi_cmd_eq_create"
3070 "Failed for EQ\n");
3071 goto create_eq_error;
3072 }
3073 
3074 beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
3075 "BM_%d : eqid = %d\n",
3076 phwi_context->be_eq[i].q.id);
3077 }
3078 return 0;
3079 
3080 create_eq_error:
3081 for (i = 0; i < (phba->num_cpus + eq_for_mcc); i++) {
3082 eq = _context->be_eq[i].q;
3083 mem = >dma_mem;
3084 if (mem->va)
3085 pci_free_consistent(phba->pcidev, num_eq_pages
3086 * PAGE_SIZE,
3087 mem->va, mem->dma);
3088 }
3089 return ret;
3090 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188961] New: Function mvs_task_prep() returns improper values on failures

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188961

Bug ID: 188961
   Summary: Function mvs_task_prep() returns improper values on
failures
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

The function mvs_task_prep() defined in file drivers/scsi/mvsas/mv_sas.c
returns 0 on success, or non-zero values on failures. It calls function
pci_pool_alloc() and checks its return value against NULL (at line 794), and if
the return value is NULL, the control flow jumps to label "err_out_tag", cleans
allocated memory and returns variable rc. Function pci_pool_alloc() is called
after the check of variable rc, so the value of rc must be 0. As a result,
mvs_task_prep() will return 0 (indicates success) even the call to
pci_pool_alloc() fails. I think it is better to assign "-ENOMEM" to rc when
pci_pool_alloc() fails. Codes and comments related to this bug are summarised
as follows.

mvs_task_prep @@ drivers/scsi/mvsas/mv_sas.c
 711 static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int
is_tmf,
 712 struct mvs_tmf_task *tmf, int *pass)
 713 {
 ...
 719 int rc = 0;
 ...
 783 rc = mvs_tag_alloc(mvi, );
 784 if (rc)
 785 goto err_out;
 786 
 787 slot = >slot_info[tag];
 788 
 789 task->lldd_task = NULL;
 790 slot->n_elem = n_elem;
 791 slot->slot_tag = tag;
 792 
 793 slot->buf = pci_pool_alloc(mvi->dma_pool, GFP_ATOMIC, >buf_dma);
 794 if (!slot->buf)
 // insert "rc = -ENOMEM" here?
 795 goto err_out_tag;
 ...
 838 return rc;
 839 
 840 err_out_slot_buf:
 841 pci_pool_free(mvi->dma_pool, slot->buf, slot->buf_dma);
 842 err_out_tag:
 843 mvs_tag_free(mvi, tag);
 844 err_out:
 845 
 846 dev_printk(KERN_ERR, mvi->dev, "mvsas prep failed[%d]!\n", rc);
 847 if (!sas_protocol_ata(task->task_proto))
 848 if (n_elem)
 849 dma_unmap_sg(mvi->dev, task->scatter, n_elem,
 850  task->data_dir);
 851 prep_out:
 852 return rc;
 853 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 189001] New: Function twl_probe() does not set error codes on some failures

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=189001

Bug ID: 189001
   Summary: Function twl_probe() does not set error codes on some
failures
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

In function twl_probe() defined in file drivers/scsi/3w-sas.c, because variable
retval is checked at line 1608, its value must be 0 when pci_iomap() is called
(at line 1614). If pci_iomap() returns a NULL pointer, the control flow jumps
to label "out_release_mem_region", cleans and returns the value of retval (i.e.
0). As a result, function twl_probe() returns 0 (indicates success) even if
there are errors. The behavior of its caller may be misled.
There are other 2 similar bugs when the function calls fail at lines 1601 and
1624. Though these errors may occur rarely, I think it is better to set the
correct error codes on failures. Codes are summarised as follows.

twl_probe @@ drivers/scsi/3w-sas.c
1564 static int twl_probe(struct pci_dev *pdev, const struct pci_device_id
*dev_id)
1565 {
1566 struct Scsi_Host *host = NULL;
1567 TW_Device_Extension *tw_dev;
1568 int retval = -ENODEV;
1569 int *ptr_phycount, phycount=0;
1570 
1571 retval = pci_enable_device(pdev);
1572 if (retval) {
1573 TW_PRINTK(host, TW_DRIVER, 0x17, "Failed to enable pci device");
1574 goto out_disable_device;
1575 }
 ...
1601 if (twl_initialize_device_extension(tw_dev)) {
1602 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1a, "Failed to initialize
device extension");
 // Bug (1): retval takes value 0. Insert "retval = -ENODEV;"?
1603 goto out_free_device_extension;
1604 }
1605 
1606 /* Request IO regions */
1607 retval = pci_request_regions(pdev, "3w-sas");
1608 if (retval) {
1609 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1b, "Failed to get mem
region");
1610 goto out_free_device_extension;
1611 }
1612 
1613 /* Save base address, use region 1 */
1614 tw_dev->base_addr = pci_iomap(pdev, 1, 0);
1615 if (!tw_dev->base_addr) {
1616 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1c, "Failed to ioremap");
 // Bug (2): retval takes value 0. Insert "retval = -ENOMEM;"?
1617 goto out_release_mem_region;
1618 }
1619 
1620 /* Disable interrupts on the card */
1621 TWL_MASK_INTERRUPTS(tw_dev);
1622 
1623 /* Initialize the card */
1624 if (twl_reset_sequence(tw_dev, 0)) {
1625 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x1d, "Controller reset failed
during probe");
 // Bug (3): retval takes value 0. Insert "retval = -ENODEV;"?
1626 goto out_iounmap;
1627 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188941] New: Function beiscsi_create_cqs() may return improper value when the call to pci_alloc_consistent() fails, which may result in use-after-free

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188941

Bug ID: 188941
   Summary: Function beiscsi_create_cqs() may return improper
value when the call to pci_alloc_consistent() fails,
which may result in use-after-free
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

Function pci_alloc_consistent() returns a NULL pointer if there is no enough
memory. In function beiscsi_create_cqs() defined in file
drivers/scsi/be2iscsi/be_main.c, function pci_alloc_consistent() is called and
its return value is checked against NULL (at line 3116). If the return value is
NULL, the control flow will jump to label "create_cq_error", frees allocated
memory and returns variable ret. Because after the first execution of the loop
the value of ret must be 0 (see the check statement of ret at line 3129), the
return value will be 0 (indicates success) if pci_alloc_consistent() fails
during the second or after repeats of the loop body. In this case, the freed
memory may be used or freed again in the callers of beiscsi_create_cqs(). I
think it is better to assign "-ENOMEM" when the call pci_alloc_consistent()
fails. Codes and comments related to this bug are summarised as follows.

beiscsi_create_cqs @@ drivers/scsi/be2iscsi/be_main.c
3092 static int beiscsi_create_cqs(struct beiscsi_hba *phba,
3093  struct hwi_context_memory *phwi_context)
3094 {
 ...
3100 int ret = -ENOMEM;
 ...
3106 for (i = 0; i < phba->num_cpus; i++) {
3107 cq = _context->be_cq[i];
3108 eq = _context->be_eq[i].q;
3109 pbe_eq = _context->be_eq[i];
3110 pbe_eq->cq = cq;
3111 pbe_eq->phba = phba;
3112 mem = >dma_mem;
3113 cq_vaddress = pci_alloc_consistent(phba->pcidev,
3114num_cq_pages * PAGE_SIZE,
3115);
3116 if (!cq_vaddress)
 // ret may takes value 0. Add "ret = -ENOMEM" here?
3117 goto create_cq_error;
 ...
3129 ret = beiscsi_cmd_cq_create(>ctrl, cq, eq, false,
3130 false, 0);
3131 if (ret) {
3132 beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
3133 "BM_%d : beiscsi_cmd_eq_create"
3134 "Failed for ISCSI CQ\n");
3135 goto create_cq_error;
3136 }
3137 beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
3138 "BM_%d : iscsi cq_id is %d for eq_id %d\n"
3139 "iSCSI CQ CREATED\n", cq->id, eq->id);
3140 }
3141 return 0;
3142 
3143 create_cq_error:
3144 for (i = 0; i < phba->num_cpus; i++) {
3145 cq = _context->be_cq[i];
3146 mem = >dma_mem;
3147 if (mem->va)
3148 pci_free_consistent(phba->pcidev, num_cq_pages
3149 * PAGE_SIZE,
3150 mem->va, mem->dma);
3151 }
3152 return ret;
3153 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188861] New: Function csio_config_device_caps() does not set error codes on failures

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188861

Bug ID: 188861
   Summary: Function csio_config_device_caps() does not set error
codes on failures
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

The function csio_config_device_caps() defined in file
drivers/scsi/csiostor/csio_hw.c returns the value of variable rv at the end. By
reviewing the source code of the caller of csio_config_device_caps(), we can
infer that variable rv should takes a non-zero value on failures. However,
after the check of variable rv at line 1376, its value must be 0. As a result,
0 will be returned even if the subsequent calls to csio_mb_issue() (at line
1389) or csio_mb_fw_retval() (at line 1394) fails. I guess letting rv receives
the return value of csio_hw_validate_caps() at line 1375 may be a typo. Does
the author means "retval" instead of "rv"? Codes related to this bug are
summarised as follows.

csio_config_device_caps @@ drivers/scsi/csiostor/csio_hw.c
1347 static int
1348 csio_config_device_caps(struct csio_hw *hw)
1349 {
1350 struct csio_mb  *mbp;
1351 enum fw_retval retval;
1352 int rv = -EINVAL;
1353 
1354 mbp = mempool_alloc(hw->mb_mempool, GFP_ATOMIC);
1355 if (!mbp) {
1356 CSIO_INC_STATS(hw, n_err_nomem);
1357 return -ENOMEM;
1358 }
 ...
1374 /* Validate device capabilities */
1375 rv = csio_hw_validate_caps(hw, mbp); // use "retval" instead of "rv"?
1376 if (rv != 0)
1377 goto out;
1378 
1379 /* Don't config device capabilities if already configured */
1380 if (hw->fw_state == CSIO_DEV_STATE_INIT) {
1381 rv = 0;
1382 goto out;
1383 }
1384 
1385 /* Write back desired device capabilities */
1386 csio_mb_caps_config(hw, mbp, CSIO_MB_DEFAULT_TMO, true, true,
1387 false, true, NULL);
1388 
1389 if (csio_mb_issue(hw, mbp)) {
1390 csio_err(hw, "Issue of FW_CAPS_CONFIG_CMD(w) failed!\n");
 // on this error, the return value is 0
1391 goto out;
1392 }
1393 
1394 retval = csio_mb_fw_retval(mbp);
1395 if (retval != FW_SUCCESS) {
1396 csio_err(hw, "FW_CAPS_CONFIG_CMD(w) returned %d!\n", retval);
 // on this error, the return value is 0
1397 goto out;
1398 }
1399 
1400 rv = 0;
1401 out:
1402 mempool_free(mbp, hw->mb_mempool);
1403 return rv;
1404 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188851] New: Function twa_probe() does not set error codes on failures

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188851

Bug ID: 188851
   Summary: Function twa_probe() does not set error codes on
failures
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

In function twa_probe(), variable retval takes the error code. However, the
error code is not set on some failures. I am not sure whether it is the
intention of the author. I list 3 positions that seems anomalous. (1) error
code "-ENODEV" is not assigned to retval when the call to
twa_initialize_device_extension() (at line 2041) fails; (2) error code
"-ENOMEM" is not assigned to retval when the call to ioremap() (at line 2062)
fails; (3) error code "-ENODEV" is not assigned to retval when the call to
twa_reset_sequence() (at line 2072) fails. Codes related to these bugs are
summarised as follows.

twa_probe @@ drivers/scsi/3w-9xxx.c
2003 /* This function will probe and initialize a card */
2004 static int twa_probe(struct pci_dev *pdev, const struct pci_device_id
*dev_id)
2005 {
2006 struct Scsi_Host *host = NULL;
2007 TW_Device_Extension *tw_dev;
2008 unsigned long mem_addr, mem_len;
2009 int retval = -ENODEV;
2010 
2011 retval = pci_enable_device(pdev);
2012 if (retval) {
2013 TW_PRINTK(host, TW_DRIVER, 0x34, "Failed to enable pci device");
2014 goto out_disable_device;
2015 }
 ...
2041 if (twa_initialize_device_extension(tw_dev)) {
2042 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x25, "Failed to initialize
device extension");
 // (1) The value of retval is 0. Insert "retval = -ENODEV;" here?
2043 goto out_free_device_extension;
2044 }
2045 
2046 /* Request IO regions */
2047 retval = pci_request_regions(pdev, "3w-9xxx");
2048 if (retval) {
2049 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x26, "Failed to get mem
region");
2050 goto out_free_device_extension;
2051 }
 ...
2062 tw_dev->base_addr = ioremap(mem_addr, mem_len);
2063 if (!tw_dev->base_addr) {
2064 TW_PRINTK(tw_dev->host, TW_DRIVER, 0x35, "Failed to ioremap");
 // (2) The value of retval is 0. Insert "retval = -ENOMEM;" here?
2065 goto out_release_mem_region;
2066 }
2067 
2068 /* Disable interrupts on the card */
2069 TW_DISABLE_INTERRUPTS(tw_dev);
2070 
2071 /* Initialize the card */
2072 if (twa_reset_sequence(tw_dev, 0))
 // (3) The value of retval is 0. Insert "retval = -ENODEV;" here?
2073 goto out_iounmap;
 ...
2133 return 0;
2134 
2135 out_remove_host:
2136 if (test_bit(TW_USING_MSI, _dev->flags))
2137 pci_disable_msi(pdev);
2138 scsi_remove_host(host);
2139 out_iounmap:
2140 iounmap(tw_dev->base_addr);
2141 out_release_mem_region:
2142 pci_release_regions(pdev);
2143 out_free_device_extension:
2144 twa_free_device_extension(tw_dev);
2145 scsi_host_put(host);
2146 out_disable_device:
2147 pci_disable_device(pdev);
2148 
2149 return retval;
2150 } /* End twa_probe() */

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Bug 188681] New: Function csio_hw_flash_erase_sectors() does not return correct error codes on failures

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=188681

Bug ID: 188681
   Summary: Function csio_hw_flash_erase_sectors() does not return
correct error codes on failures
   Product: SCSI Drivers
   Version: 2.5
Kernel Version: linux-4.9-rc6
  Hardware: All
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Other
  Assignee: scsi_drivers-ot...@kernel-bugs.osdl.org
  Reporter: bianpan2...@ruc.edu.cn
Regression: No

>From the usages of function csio_hw_flash_erase_sectors() defined in
drivers/scsi/csiostor/csio_hw.c, we can infer that it should return a non-zero
error code when something goes wrong. However, it will return 0 even the calls
to csio_hw_sf1_write() or csio_hw_flash_wait_op() fails. Maybe it is better to
use "return ret;" instead of "return 0;" at line 616. Codes related to this bug
are summarised as follows.

csio_hw_flash_erase_sectors @@ drivers/scsi/csiostor/csio_hw.c
 589 static int
 590 csio_hw_flash_erase_sectors(struct csio_hw *hw, int32_t start, int32_t
end)
 591 {
 592 int ret = 0;
 593 
 594 while (start <= end) {
 595 
 596 ret = csio_hw_sf1_write(hw, 1, 0, 1, SF_WR_ENABLE);
 597 if (ret != 0)
 598 goto out;
 599 
 600 ret = csio_hw_sf1_write(hw, 4, 0, 1,
 601 SF_ERASE_SECTOR | (start << 8));
 602 if (ret != 0)
 603 goto out;
 604 
 605 ret = csio_hw_flash_wait_op(hw, 14, 500);
 606 if (ret != 0)
 607 goto out;
 608 
 609 start++;
 610 }
 611 out:
 612 if (ret)
 613 csio_err(hw, "erase of flash sector %d failed, error %d\n",
 614  start, ret);
 615 csio_wr_reg32(hw, 0, SF_OP_A);/* unlock SF */
 616 return 0;  // return ret?
 617 }

csio_hw_fw_dload @@ drivers/scsi/csiostor/csio_hw.c
 667 static int
 668 csio_hw_fw_dload(struct csio_hw *hw, uint8_t *fw_data, uint32_t size)
 669 {
 ...
 719 ret = csio_hw_flash_erase_sectors(hw, FLASH_FW_START_SEC,
 720   FLASH_FW_START_SEC + i - 1);
 721 if (ret) {   // check the return value of
csio_hw_flash_erase_sectors()
 722 csio_err(hw, "Flash Erase failed\n");
 723 goto out;
 724 }
 ...
 755 out:
 756 if (ret)
 757 csio_err(hw, "firmware download failed, error %d\n", ret);
 758 return ret;
 759 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] scsi: libfc: Remove an unneeded condition

[Johannes Thumshirn]

On Thu, Nov 24, 2016 at 01:52:38PM +0300, Dan Carpenter wrote:
> We verified that resp_code is FC_SPP_RESP_ACK earlier so we don't need
> to check again here.
> 
> Signed-off-by: Dan Carpenter 
> 

Looks good,
Acked-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: SG does not ignore dxferp (direct io + mmap)

[Johannes Thumshirn]

On Wed, Nov 23, 2016 at 03:22:04PM -0500, Ewan Milne wrote:

[...]

> ---
> 
> In other words, this commit made the bad behavior go away in 4.8.
> Need to look at this in more detail, it doesn't appear as if this patch
> was intended to fix such a problem.
> 
> -Ewan

Are you sure it did? I can repropduce copy_to_user() errors with 4.8 as well.
Using the very same reproducer. On 4.8 it's just harder to trigger and
doesn't trigger on AHCI as fas as I can telli (maybe I just haven't hit
it hard enough). I can trigger it on QEMUs SCSI CDROM emulation and hpsa
though. I cannot however trigger this with a minimal config inside an initrd.


Here's my test:

--- 8< ---
#!/bin/sh 

cleanup()
{
rm -f expect.txt
rm -f test.txt
}

trap cleanup EXIT INT 

./sg_mmap_read -d /dev/sg0 -l 0 | od -x > expect.txt

for i in $(seq 1 5000); do
./sg_mmap_read -d /dev/sg0 -l 0 | od -x > test.txt
diff expect.txt test.txt > /dev/null
if [ $? -ne 0 ]; then
echo "FAIL"
exit 1
fi
done

echo "PASS"

--- >8 ---

I added some trace_printk()s to sg.c and try to hunt it down. It'll be good to
know if anyone of you can reproduce it with the above script as well.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html