[PATCH][RESEND] block: sed-opal: fix u64 short atom length

[PATCH][RESEND] block: sed-opal: fix u64 short atom length

[Jonas Rabenstein]

The length must be given as bytes and not as 4 bit tuples.

Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
---
 block/sed-opal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 36842bfa572e..d5f565e1557a 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -562,7 +562,7 @@ static void add_token_u64(int *err, struct opal_dev *cmd, u64 number)
 	}
 
 	msb = fls(number);
-	len = DIV_ROUND_UP(msb, 4);
+	len = DIV_ROUND_UP(msb, 8);
 
 	if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
 		pr_debug("Error adding u64: end of buffer.\n");
-- 
2.13.6

Re: [PATCH][RESEND] block: sed-opal: fix u64 short atom length

[Derrick, Jonathan]

[-- Attachment #1: Type: text/plain, Size: 965 bytes --]

Hi Jonas,

On Thu, 2018-03-01 at 14:27 +0100, Jonas Rabenstein wrote:
> The length must be given as bytes and not as 4 bit tuples.
> 
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlange
> n.de>
> ---
>  block/sed-opal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/sed-opal.c b/block/sed-opal.c
> index 36842bfa572e..d5f565e1557a 100644
> --- a/block/sed-opal.c
> +++ b/block/sed-opal.c
> @@ -562,7 +562,7 @@ static void add_token_u64(int *err, struct
> opal_dev *cmd, u64 number)
>  	}
>  
>  	msb = fls(number);
> -	len = DIV_ROUND_UP(msb, 4);
> +	len = DIV_ROUND_UP(msb, 8);

This change looks partially correct, but I believe we should be doing
fls64() on 'number' as well.

It looks like it currently coincidentally works with u64 numbers
falling in 32-bit ranges.


>  
>  	if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
>  		pr_debug("Error adding u64: end of buffer.\n");

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]

[PATCH v2] block: sed-opal: fix u64 short atom length

[Jonas Rabenstein]

The length must be given as bytes and not as 4 bit tuples.

Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
---
v2:
  - use fls64
  - shorten loop body
---
 block/sed-opal.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 36842bfa572e..38411c5c477f 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -554,15 +554,14 @@ static void add_token_u64(int *err, struct opal_dev *cmd, u64 number)
 
 	size_t len;
 	int msb;
-	u8 n;
 
 	if (!(number & ~TINY_ATOM_DATA_MASK)) {
 		add_token_u8(err, cmd, number);
 		return;
 	}
 
-	msb = fls(number);
-	len = DIV_ROUND_UP(msb, 4);
+	msb = fls64(number);
+	len = DIV_ROUND_UP(msb, 8);
 
 	if (cmd->pos >= IO_BUFFER_LENGTH - len - 1) {
 		pr_debug("Error adding u64: end of buffer.\n");
@@ -570,10 +569,8 @@ static void add_token_u64(int *err, struct opal_dev *cmd, u64 number)
 		return;
 	}
 	add_short_atom_header(cmd, false, false, len);
-	while (len--) {
-		n = number >> (len * 8);
-		add_token_u8(err, cmd, n);
-	}
+	while (len--)
+		add_token_u8(err, cmd, number >> (len * 8));
 }
 
 static void add_token_bytestring(int *err, struct opal_dev *cmd,
-- 
2.16.1

Re: [PATCH v2] block: sed-opal: fix u64 short atom length

[Scott Bauer]

On Wed, Mar 07, 2018 at 05:55:56PM +0100, Jonas Rabenstein wrote:
> The length must be given as bytes and not as 4 bit tuples.
> 
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> ---
> v2:
>   - use fls64
>   - shorten loop body
> ---
>  block/sed-opal.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 

Reviewed-by: Scott Bauer <scott.bauer@intel.com>

Your two patches should be sent to stable for 4.14. I can queue those up and do it,
or if you want to you can do it as well. Let me know what you prefer!

Re: [PATCH v2] block: sed-opal: fix u64 short atom length

[Jonas Rabenstein]

On Wed, Mar 07, 2018 at 04:24:29PM -0700, Scott Bauer wrote:
> On Wed, Mar 07, 2018 at 05:55:56PM +0100, Jonas Rabenstein wrote:
> > The length must be given as bytes and not as 4 bit tuples.
> > 
> > Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> > ---
> > v2:
> >   - use fls64
> >   - shorten loop body
> > ---
> >  block/sed-opal.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> 
> Reviewed-by: Scott Bauer <scott.bauer@intel.com>
> 
> Your two patches should be sent to stable for 4.14. I can queue those up and do it,
> or if you want to you can do it as well. Let me know what you prefer!
As I am quite new to all that kernel patch submitting stuff, I would be
glad if you could do it for me so I will not mess it up (;

Thanks,
 Jonas

Re: [PATCH v2] block: sed-opal: fix u64 short atom length

[Scott Bauer]

On Wed, Mar 07, 2018 at 05:55:56PM +0100, Jonas Rabenstein wrote:
> The length must be given as bytes and not as 4 bit tuples.
> 
> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
> ---
> v2:
>   - use fls64
>   - shorten loop body
> ---
>  block/sed-opal.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Scott Bauer <scott.bauer@intel.com>
Tested-by: Scott Bauer <scott.bauer@intel.com>

Hi Jens,

When you get time can you apply this if you have no objections?

Thanks

Re: [PATCH v2] block: sed-opal: fix u64 short atom length

[Jens Axboe]

On 3/16/18 8:38 AM, Scott Bauer wrote:
> On Wed, Mar 07, 2018 at 05:55:56PM +0100, Jonas Rabenstein wrote:
>> The length must be given as bytes and not as 4 bit tuples.
>>
>> Signed-off-by: Jonas Rabenstein <jonas.rabenstein@studium.uni-erlangen.de>
>> ---
>> v2:
>>   - use fls64
>>   - shorten loop body
>> ---
>>  block/sed-opal.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> Reviewed-by: Scott Bauer <scott.bauer@intel.com>
> Tested-by: Scott Bauer <scott.bauer@intel.com>
> 
> Hi Jens,
> 
> When you get time can you apply this if you have no objections?

Done.

-- 
Jens Axboe