Re: [PATCH] scsi: libiscsi: select CRYPTO_HASH for ISCSI_TCP

2021-10-15 Thread Chris Leech
On Fri, Oct 15, 2021 at 6:54 AM Vegard Nossum  wrote:
>
>
> On 10/15/21 3:36 PM, Herbert Xu wrote:
> > On Fri, Oct 15, 2021 at 03:11:15PM +0200, Vegard Nossum wrote:
> >> Fix the following build/link error by adding a dependency on CRYPTO_HASH:
> >>
> >>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_dgst_header':
> >>   libiscsi_tcp.c:(.text+0x237): undefined reference to 
> >> `crypto_ahash_digest'
> >>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_segment_done':
> >>   libiscsi_tcp.c:(.text+0x1325): undefined reference to 
> >> `crypto_ahash_final'
> >>
> >> Fixes: 5d6ac29b9ebf2 ("iscsi_tcp: Use ahash")
> >> Cc: Herbert Xu 
> >> Signed-off-by: Vegard Nossum 
> >> ---
> >>  drivers/scsi/Kconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> >> index 6e3a04107bb65..09764f3c42447 100644
> >> --- a/drivers/scsi/Kconfig
> >> +++ b/drivers/scsi/Kconfig
> >> @@ -290,6 +290,7 @@ config ISCSI_TCP
> >>  tristate "iSCSI Initiator over TCP/IP"
> >>  depends on SCSI && INET
> >>  select CRYPTO
> >> +select CRYPTO_HASH
> >>  select CRYPTO_MD5
> >>  select CRYPTO_CRC32C
> >>  select SCSI_ISCSI_ATTRS
> >
> > CRYPTO_MD5 already selects CRYPTO_HASH so this shouldn't be needed.
>
> You're right, my apologies.
>
> libiscsi_tcp.o is built for CONFIG_ISCSI_TCP, but it's _also_ built for
> CONFIG_SCSI_CXGB3_ISCSI and CONFIG_SCSI_CXGB4_ISCSI. I missed that when
> I looked at the Makefile and wrongly assumed that CONFIG_ISCSI_TCP was
> missing the select.
>
> I'm not sure what the right fix here would be -- should
> CONFIG_SCSI_CXGB[34]_ISCSI depend on CONFIG_ISCSI_TCP..? Or should we
> add those CRYPTO* selects in there as well?

(apologies for the HTML reply that got rejected by the lists, and the
double reply to those directly addressed)

Are  Kconfig entries for libiscsi and libiscsi_tcp needed to handle
this properly?  Currently they're added to the object lists with each
configured driver that requires them.  I'm not even sure what happens
if you manage to configure them into both obj-y and obj-m.

- Chris Leech

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/CAPnfmXJPQTcZmPkYVDL4feDH7_P742N1H885suQzJc6u2A9QYQ%40mail.gmail.com.


Re: [PATCH] scsi: libiscsi: select CRYPTO_HASH for ISCSI_TCP

2021-10-15 Thread Chris Leech
On Fri, Oct 15, 2021 at 6:54 AM Vegard Nossum 
wrote:

>
> On 10/15/21 3:36 PM, Herbert Xu wrote:
> > On Fri, Oct 15, 2021 at 03:11:15PM +0200, Vegard Nossum wrote:
> >> Fix the following build/link error by adding a dependency on
> CRYPTO_HASH:
> >>
> >>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_dgst_header':
> >>   libiscsi_tcp.c:(.text+0x237): undefined reference to
> `crypto_ahash_digest'
> >>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_segment_done':
> >>   libiscsi_tcp.c:(.text+0x1325): undefined reference to
> `crypto_ahash_final'
> >>
> >> Fixes: 5d6ac29b9ebf2 ("iscsi_tcp: Use ahash")
> >> Cc: Herbert Xu 
> >> Signed-off-by: Vegard Nossum 
> >> ---
> >>  drivers/scsi/Kconfig | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> >> index 6e3a04107bb65..09764f3c42447 100644
> >> --- a/drivers/scsi/Kconfig
> >> +++ b/drivers/scsi/Kconfig
> >> @@ -290,6 +290,7 @@ config ISCSI_TCP
> >>  tristate "iSCSI Initiator over TCP/IP"
> >>  depends on SCSI && INET
> >>  select CRYPTO
> >> +select CRYPTO_HASH
> >>  select CRYPTO_MD5
> >>  select CRYPTO_CRC32C
> >>  select SCSI_ISCSI_ATTRS
> >
> > CRYPTO_MD5 already selects CRYPTO_HASH so this shouldn't be needed.
>
> You're right, my apologies.
>
> libiscsi_tcp.o is built for CONFIG_ISCSI_TCP, but it's _also_ built for
> CONFIG_SCSI_CXGB3_ISCSI and CONFIG_SCSI_CXGB4_ISCSI. I missed that when
> I looked at the Makefile and wrongly assumed that CONFIG_ISCSI_TCP was
> missing the select.
>
> I'm not sure what the right fix here would be -- should
> CONFIG_SCSI_CXGB[34]_ISCSI depend on CONFIG_ISCSI_TCP..? Or should we
> add those CRYPTO* selects in there as well?
>

Are  Kconfig entries for libiscsi and libiscsi_tcp needed to handle this
properly?  Currently they're added to the object lists with each configured
driver that requires them.  I'm not even sure what happens if you manage to
configure them into both obj-y and obj-m.

- Chris Leech

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/CAPnfmXJQCEH3KpECqPoOU%3D73amqe0ubnx9Z36b1Ra0Yrg8_gUg%40mail.gmail.com.


[PATCH] scsi: libiscsi: select CRYPTO_HASH for ISCSI_TCP

2021-10-15 Thread Vegard Nossum
Fix the following build/link error by adding a dependency on CRYPTO_HASH:

  ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_dgst_header':
  libiscsi_tcp.c:(.text+0x237): undefined reference to `crypto_ahash_digest'
  ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_segment_done':
  libiscsi_tcp.c:(.text+0x1325): undefined reference to `crypto_ahash_final'

Fixes: 5d6ac29b9ebf2 ("iscsi_tcp: Use ahash")
Cc: Herbert Xu 
Signed-off-by: Vegard Nossum 
---
 drivers/scsi/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 6e3a04107bb65..09764f3c42447 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -290,6 +290,7 @@ config ISCSI_TCP
tristate "iSCSI Initiator over TCP/IP"
depends on SCSI && INET
select CRYPTO
+   select CRYPTO_HASH
select CRYPTO_MD5
select CRYPTO_CRC32C
select SCSI_ISCSI_ATTRS
-- 
2.23.0.718.g5ad94255a8

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/20211015131115.12720-1-vegard.nossum%40oracle.com.


Re: [PATCH] scsi: libiscsi: select CRYPTO_HASH for ISCSI_TCP

2021-10-15 Thread Vegard Nossum


On 10/15/21 3:36 PM, Herbert Xu wrote:
> On Fri, Oct 15, 2021 at 03:11:15PM +0200, Vegard Nossum wrote:
>> Fix the following build/link error by adding a dependency on CRYPTO_HASH:
>>
>>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_dgst_header':
>>   libiscsi_tcp.c:(.text+0x237): undefined reference to `crypto_ahash_digest'
>>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_segment_done':
>>   libiscsi_tcp.c:(.text+0x1325): undefined reference to `crypto_ahash_final'
>>
>> Fixes: 5d6ac29b9ebf2 ("iscsi_tcp: Use ahash")
>> Cc: Herbert Xu 
>> Signed-off-by: Vegard Nossum 
>> ---
>>  drivers/scsi/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index 6e3a04107bb65..09764f3c42447 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -290,6 +290,7 @@ config ISCSI_TCP
>>  tristate "iSCSI Initiator over TCP/IP"
>>  depends on SCSI && INET
>>  select CRYPTO
>> +select CRYPTO_HASH
>>  select CRYPTO_MD5
>>  select CRYPTO_CRC32C
>>  select SCSI_ISCSI_ATTRS
> 
> CRYPTO_MD5 already selects CRYPTO_HASH so this shouldn't be needed.

You're right, my apologies.

libiscsi_tcp.o is built for CONFIG_ISCSI_TCP, but it's _also_ built for
CONFIG_SCSI_CXGB3_ISCSI and CONFIG_SCSI_CXGB4_ISCSI. I missed that when
I looked at the Makefile and wrongly assumed that CONFIG_ISCSI_TCP was
missing the select.

I'm not sure what the right fix here would be -- should
CONFIG_SCSI_CXGB[34]_ISCSI depend on CONFIG_ISCSI_TCP..? Or should we
add those CRYPTO* selects in there as well?


Vegard

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/3bba3fed-b8f8-b7ce-f26f-8d1ed221144a%40oracle.com.


Re: [PATCH] scsi: libiscsi: select CRYPTO_HASH for ISCSI_TCP

2021-10-15 Thread Herbert Xu
On Fri, Oct 15, 2021 at 03:11:15PM +0200, Vegard Nossum wrote:
> Fix the following build/link error by adding a dependency on CRYPTO_HASH:
> 
>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_dgst_header':
>   libiscsi_tcp.c:(.text+0x237): undefined reference to `crypto_ahash_digest'
>   ld: drivers/scsi/libiscsi_tcp.o: in function `iscsi_tcp_segment_done':
>   libiscsi_tcp.c:(.text+0x1325): undefined reference to `crypto_ahash_final'
> 
> Fixes: 5d6ac29b9ebf2 ("iscsi_tcp: Use ahash")
> Cc: Herbert Xu 
> Signed-off-by: Vegard Nossum 
> ---
>  drivers/scsi/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 6e3a04107bb65..09764f3c42447 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -290,6 +290,7 @@ config ISCSI_TCP
>   tristate "iSCSI Initiator over TCP/IP"
>   depends on SCSI && INET
>   select CRYPTO
> + select CRYPTO_HASH
>   select CRYPTO_MD5
>   select CRYPTO_CRC32C
>   select SCSI_ISCSI_ATTRS

CRYPTO_MD5 already selects CRYPTO_HASH so this shouldn't be needed.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/20211015133635.GA26418%40gondor.apana.org.au.


Re: Antw: [EXT] Re: [PATCH] scsi scsi_transport_iscsi.c: fix misuse of %llu in scsi_transport_iscsi.c

2021-10-15 Thread Guo Zhi

On 2021/10/11 23:29, Mike Christie wrote:

On 10/11/21 1:35 AM, Ulrich Windl wrote:

Joe Perches  schrieb am 09.10.2021 um 05:14 in Nachricht

<5daf69b365e23ceecee911c4d0f2f66a0b9ec95c.ca...@perches.com>:

On Sat, 2021-10-09 at 11:02 +0800, Guo Zhi wrote:

Pointers should be printed with %p or %px rather than
cast to (unsigned long long) and printed with %llu.
Change %llu to %p to print the pointer into sysfs.

][]

diff --git a/drivers/scsi/scsi_transport_iscsi.c

b/drivers/scsi/scsi_transport_iscsi.c
[]

@@ -129,8 +129,8 @@ show_transport_handle(struct device *dev, struct

device_attribute *attr,
  


if (!capable(CAP_SYS_ADMIN))
return -EACCES;
-   return sysfs_emit(buf, "%llu\n",
- (unsigned long long)iscsi_handle(priv->iscsi_transport));
+   return sysfs_emit(buf, "%p\n",
+   iscsi_ptr(priv->iscsi_transport));

iscsi_transport is a pointer isn't it?

so why not just

return sysfs_emit(buf, "%p\n", priv->iscsi_transport);

Isn't the difference that %p outputs hex, while %u outputs decimal?


Yeah, I think this patch will break userspace, because it doesn't know it's
a pointer. It could be doing:

sscanf(str, "%llu", );

The value is just later passed back to the kernel to look up a driver in
iscsi_if_transport_lookup:

 list_for_each_entry(priv, _transports, list) {
 if (tt == priv->iscsi_transport) {

so we could just replace priv->transport with an int and use an ida to assign
the value.


Taking security into consideration, We should not print kernel pointer 
into sysfs.


However if this is a special pointer to lookup a driver,  It's really 
tricky for me to fix it,


as I don't have a scsi device to test my code.


Guo


--
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/6772c5ef-4666-e2b5-2885-797baa939b45%40sjtu.edu.cn.