Re: [PATCH v3 7/8] scsi/pmcraid: Remove an unused structure member

[Hannes Reinecke]

On 11/03/2017 11:23 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: linux-scsi@vger.kernel.org
> Cc: Martin K. Petersen 
> Cc: Anil Ravindranath 
> ---
>  drivers/scsi/pmcraid.h | 1 -
>  1 file changed, 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH v3 8/8] scsi/pmcraid: Use sgl_alloc_order() and sgl_free_order()

[Hannes Reinecke]

On 11/03/2017 11:23 PM, Bart Van Assche wrote:
> Use the sgl_alloc_order() and sgl_free_order() functions instead
> of open coding these functions.
> 
> Signed-off-by: Bart Van Assche 
> Reviewed-by: Johannes Thumshirn 
> Cc: linux-scsi@vger.kernel.org
> Cc: Martin K. Petersen 
> Cc: Anil Ravindranath 
> ---
>  drivers/scsi/Kconfig   |  1 +
>  drivers/scsi/pmcraid.c | 43 ---
>  drivers/scsi/pmcraid.h |  2 +-
>  3 files changed, 6 insertions(+), 40 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

RE: [PATCH v2] scsi: be2iscsi: Use kasprintf

[Jitendra Bhivare]

> -Original Message-
> From: Himanshu Jha [mailto:himanshujha199...@gmail.com]
> Sent: Wednesday, October 11, 2017 9:06 PM
> To: j...@linux.vnet.ibm.com
> Cc: martin.peter...@oracle.com; linux-scsi@vger.kernel.org; linux-
> ker...@vger.kernel.org; subbu.seethara...@broadcom.com;
> ketan.muka...@broadcom.com; jitendra.bhiv...@broadcom.com;
> Himanshu Jha 
> Subject: [PATCH v2] scsi: be2iscsi: Use kasprintf
>
> Use kasprintf instead of combination of kmalloc and sprintf.
> Also, remove BEISCSI_MSI_NAME macro used to specify size of string as
> kasprintf handles size computations.
>
> Signed-off-by: Himanshu Jha 
> ---
> v2:
>-remove the unnecessary macro BEISCSI_MSI_NAME.
>
>  drivers/scsi/be2iscsi/be_main.c | 12 +---
> drivers/scsi/be2iscsi/be_main.h |  2 --
>  2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c
b/drivers/scsi/be2iscsi/be_main.c
> index b4542e7..6a9ee0e 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -803,15 +803,14 @@ static int beiscsi_init_irqs(struct beiscsi_hba
*phba)
>
>   if (pcidev->msix_enabled) {
>   for (i = 0; i < phba->num_cpus; i++) {
> - phba->msi_name[i] = kzalloc(BEISCSI_MSI_NAME,
> - GFP_KERNEL);
> + phba->msi_name[i] = kasprintf(GFP_KERNEL,
> +   "beiscsi_%02x_%02x",
> +
phba->shost->host_no, i);
>   if (!phba->msi_name[i]) {
>   ret = -ENOMEM;
>   goto free_msix_irqs;
>   }
>
> - sprintf(phba->msi_name[i], "beiscsi_%02x_%02x",
> - phba->shost->host_no, i);
>   ret = request_irq(pci_irq_vector(pcidev, i),
> be_isr_msix, 0,
phba->msi_name[i],
> _context->be_eq[i]);
> @@ -824,13 +823,12 @@ static int beiscsi_init_irqs(struct beiscsi_hba
*phba)
>   goto free_msix_irqs;
>   }
>   }
> - phba->msi_name[i] = kzalloc(BEISCSI_MSI_NAME,
> GFP_KERNEL);
> + phba->msi_name[i] = kasprintf(GFP_KERNEL,
> "beiscsi_mcc_%02x",
> +   phba->shost->host_no);
>   if (!phba->msi_name[i]) {
>   ret = -ENOMEM;
>   goto free_msix_irqs;
>   }
> - sprintf(phba->msi_name[i], "beiscsi_mcc_%02x",
> - phba->shost->host_no);
>   ret = request_irq(pci_irq_vector(pcidev, i), be_isr_mcc,
0,
> phba->msi_name[i], _context-
> >be_eq[i]);
>   if (ret) {
> diff --git a/drivers/scsi/be2iscsi/be_main.h
b/drivers/scsi/be2iscsi/be_main.h
> index 81ce3ff..8166de5 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -155,8 +155,6 @@
>  #define PAGES_REQUIRED(x) \
>   ((x < PAGE_SIZE) ? 1 :  ((x + PAGE_SIZE - 1) / PAGE_SIZE))
>
> -#define BEISCSI_MSI_NAME 20 /* size of msi_name string */
> -
>  #define MEM_DESCR_OFFSET 8
>  #define BEISCSI_DEFQ_HDR 1
>  #define BEISCSI_DEFQ_DATA 0
> --
> 2.7.4

Looks good.

- Thanks.

Reviewed-by: Jitendra Bhivare 

Re: [PATCH 0/4] scsi: qla2xxx: Convert timers to use timer_setup()

[Bart Van Assche]

On Wed, 2017-11-01 at 19:18 +, Madhani, Himanshu wrote:
> Hi Kees, 
> 
> > On Nov 1, 2017, at 11:46 AM, Kees Cook  wrote:
> > 
> > On Tue, Oct 31, 2017 at 12:13 PM, Kees Cook  wrote:
> > > This breaks out the logical steps to convert the qla2xxx timers:
> > > 
> > > 1) init_timer() -> setup_timer()
> > > 2) refactor qla2x00_start_timer() to not pass callback as argument
> > > 3) qla2x00_timer() to use timer_setup()
> > > 4) qla2x00_sp_timeout() to use timer_setup()
> > > 
> > > The resulting diff is identical to the patch that appears to lock up
> > > the driver. This should help identify which step causes this behavior.
> > 
> > Hi, just curious if there's been any progress on debugging the issue
> > you saw with this series?
> 
> Sorry for delay. I will test this series later today and post update. 

Hello Himanshu and Kees,

The qla2xxx driver does not crash if apply these four patches on kernel
v4.14-rc8 and load the qla2xxx driver on my test setup. Point-to-point
mode seems broken again but I don't think that's related to this patch
series.

Bart.

Re: target-pending/for-next patches

[James Bottomley]

On Sat, 2017-11-04 at 18:14 -0700, Nicholas A. Bellinger wrote:
> Hi all,
> 
> Just a friendly email after catching up on patches this week, the
> majority of those outstanding on the list have been merged into
> target-pending/for-next.  Please see below.
> 
> For those who submitted patches, please have a look and let me know
> if anything is else missing.  Note there are two exceptions that have
> been left out for now that I'll be following up with separately.
> 
> Thus far it's all been either straight-forward bug-fixes, minor
> cleanups, or small miscellaneous enhancements.  AFAICT, nothing looks
> particularly concerning.

The concern would be you dumping a tree on the eve of a merge window,
which you're presumably going to send to Linus in a week or so, when
the last time you appeared was a fixes pull in 12 August, because it
suggests this lot is just some randomly chosen selection to try to keep
the tree alive.  I really wouldn't do it like this: I know Linus
doesn't care too much for SCSI stuff and if you're lucky he may be too
busy yelling at Jens to notice, but if not, you'll find yourself on the
receiving end of his ire and that will damage the reputation of your
tree a lot.

If the work of running the target tree has got too much, get a patch
wrangler who can help with the process stuff you're completely lacking,
like reviews and testing and long incubation in linux-next for exposure
to 0day.  I'm sure we can find several volunteers.

James

[PATCH] scsi: advansys: fix improper function call to kfree

[Pan Bian]

In function advansys_eisa_probe(), data->host[i] holds the return value
of scsi_host_alloc(). The memory allocated by scsi_host_alloc() should
be deallocated with scsi_host_put(), not kfree().

Signed-off-by: Pan Bian 
---
 drivers/scsi/advansys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 24e57e7..1f56a6d 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -11678,8 +11678,8 @@ static int advansys_eisa_probe(struct device *dev)
return 0;
 
  free_data:
-   kfree(data->host[0]);
-   kfree(data->host[1]);
+   scsi_host_put(data->host[0]);
+   scsi_host_put(data->host[1]);
kfree(data);
  fail:
return err;
-- 
1.9.1

Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface

[Greg KH]

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Sun, Nov 05, 2017 at 08:11:20PM +1100, Aleksa Sarai wrote:
> I've booted it on a few of my laptops, and nothing seemed to break. Is
> there a particular test-suite you'd recommend that I run?

Workloads/tools that normally interact with this file would be the best
ones, right?  Odds are, your normal "laptop booting/running" mode does
not do anything with this file.

thanks,

greg k-h

Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface

[Aleksa Sarai]

I've booted it on a few of my laptops, and nothing seemed to break. Is
there a particular test-suite you'd recommend that I run?

On Sun, Nov 5, 2017 at 6:31 PM, Greg KH  wrote:
> On Sun, Nov 05, 2017 at 01:56:35PM +1100, Aleksa Sarai wrote:
>> Previously, the only capability effectively required to operate on the
>> /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files,
>> having an fsuid of GLOBAL_ROOT_UID was enough). This means that
>> semi-privileged processes could interfere with core components of a
>> system (such as causing a DoS by removing the underlying SCSI device of
>> the host's / mount).
>
> Given that the previous patch didn't even compile, I worry that you have
> not tested this at all to see what breaks/changes in userspace with this
> type of user-visable api change.
>
> What did you do to test this?
>
> thanks,
>
> greg k-h



-- 
Aleksa Sarai (cyphar)
www.cyphar.com

Re: [PATCH v3] scsi: require CAP_SYS_ADMIN to write to procfs interface

[Greg KH]

On Sun, Nov 05, 2017 at 01:56:35PM +1100, Aleksa Sarai wrote:
> Previously, the only capability effectively required to operate on the
> /proc/scsi interface was CAP_DAC_OVERRIDE (or for some other files,
> having an fsuid of GLOBAL_ROOT_UID was enough). This means that
> semi-privileged processes could interfere with core components of a
> system (such as causing a DoS by removing the underlying SCSI device of
> the host's / mount).

Given that the previous patch didn't even compile, I worry that you have
not tested this at all to see what breaks/changes in userspace with this
type of user-visable api change.

What did you do to test this?

thanks,

greg k-h