[SRU][Xenial][PATCH 0/1] nbd: use loff_t for blocksize and nbd_set_size args

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 0/1] nbd: use loff_t for blocksize and nbd_set_size args

Joseph Salisbury-3
BugLink: http://bugs.launchpad.net/bugs/1772575

== SRU Justification ==
With the Xenial kernel, the bug reporter states there is an NBD size
overflow with image size exceeding 1TB.

There's an issue in kernel's NBD module which prevents some larger images to be
correctly "connected" , largely described here:
https://github.com/NetworkBlockDevice/nbd/issues/44

This is a regression from Trusty and was fixed in mainline as of v4.10-rc1.

== Fix ==
ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")

== Regression Potential ==
Medium.  A backport was needed.  However, it was due to some context
diffs and the way debugfs_create_u32 was used in Xenial and how
debugfs_create_u64 was used in the patch.

== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Josef Bacik (1):
  nbd: use loff_t for blocksize and nbd_set_size args

 drivers/block/nbd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 1/1] nbd: use loff_t for blocksize and nbd_set_size args

Joseph Salisbury-3
From: Josef Bacik <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1772575

If we have large devices (say like the 40t drive I was trying to test with) we
will end up overflowing the int arguments to nbd_set_size and not get the right
size for our device.  Fix this by using loff_t everywhere so I don't have to
think about this again.  Thanks,

Signed-off-by: Josef Bacik <[hidden email]>
Signed-off-by: Jens Axboe <[hidden email]>
(back ported from commit ef77b515243b3499d62cf446eda6ca7e0a0b079c)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 drivers/block/nbd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5f807a6..673632c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -54,7 +54,7 @@ struct nbd_device {
 
  struct mutex tx_lock;
  struct gendisk *disk;
- int blksize;
+ loff_t blksize;
  loff_t bytesize;
  int xmit_timeout;
  bool disconnect; /* a disconnect has been requested by user */
@@ -135,7 +135,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
 }
 
 static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
- int blocksize, int nr_blocks)
+ loff_t blocksize, loff_t nr_blocks)
 {
  int ret;
 
@@ -144,7 +144,7 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
  return ret;
 
  nbd->blksize = blocksize;
- nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks;
+ nbd->bytesize = blocksize * nr_blocks;
 
  nbd_size_update(nbd, bdev);
 
@@ -959,7 +959,7 @@ static int nbd_dev_dbg_init(struct nbd_device *nbd)
  return PTR_ERR(f);
  }
 
- f = debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize);
+ f = debugfs_create_u64("blocksize", 0444, dir, &nbd->blksize);
  if (IS_ERR_OR_NULL(f)) {
  dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'blocksize', %ld\n",
  PTR_ERR(f));
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH 1/1] nbd: use loff_t for blocksize and nbd_set_size args

Kleber Sacilotto de Souza
On 05/23/18 10:48, Joseph Salisbury wrote:

> From: Josef Bacik <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> If we have large devices (say like the 40t drive I was trying to test with) we
> will end up overflowing the int arguments to nbd_set_size and not get the right
> size for our device.  Fix this by using loff_t everywhere so I don't have to
> think about this again.  Thanks,
>
> Signed-off-by: Josef Bacik <[hidden email]>
> Signed-off-by: Jens Axboe <[hidden email]>
> (back ported from commit ef77b515243b3499d62cf446eda6ca7e0a0b079c)
> Signed-off-by: Joseph Salisbury <[hidden email]>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  drivers/block/nbd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 5f807a6..673632c 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -54,7 +54,7 @@ struct nbd_device {
>  
>   struct mutex tx_lock;
>   struct gendisk *disk;
> - int blksize;
> + loff_t blksize;
>   loff_t bytesize;
>   int xmit_timeout;
>   bool disconnect; /* a disconnect has been requested by user */
> @@ -135,7 +135,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
>  }
>  
>  static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
> - int blocksize, int nr_blocks)
> + loff_t blocksize, loff_t nr_blocks)
>  {
>   int ret;
>  
> @@ -144,7 +144,7 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
>   return ret;
>  
>   nbd->blksize = blocksize;
> - nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks;
> + nbd->bytesize = blocksize * nr_blocks;
>  
>   nbd_size_update(nbd, bdev);
>  
> @@ -959,7 +959,7 @@ static int nbd_dev_dbg_init(struct nbd_device *nbd)
>   return PTR_ERR(f);
>   }
>  
> - f = debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize);
> + f = debugfs_create_u64("blocksize", 0444, dir, &nbd->blksize);
>   if (IS_ERR_OR_NULL(f)) {
>   dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'blocksize', %ld\n",
>   PTR_ERR(f));
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/APPLIED: [SRU][Xenial][PATCH 1/1] nbd: use loff_t for blocksize and nbd_set_size args

Juerg Haefliger
In reply to this post by Joseph Salisbury-3
On 05/23/2018 07:48 PM, Joseph Salisbury wrote:

> From: Josef Bacik <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> If we have large devices (say like the 40t drive I was trying to test with) we
> will end up overflowing the int arguments to nbd_set_size and not get the right
> size for our device.  Fix this by using loff_t everywhere so I don't have to
> think about this again.  Thanks,
>
> Signed-off-by: Josef Bacik <[hidden email]>
> Signed-off-by: Jens Axboe <[hidden email]>
> (back ported from commit ef77b515243b3499d62cf446eda6ca7e0a0b079c)
> Signed-off-by: Joseph Salisbury <[hidden email]>
Acked-by: Juerg Haefliger <[hidden email]>

Applied to xenial/master-next.

...Juerg

> ---
>  drivers/block/nbd.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 5f807a6..673632c 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -54,7 +54,7 @@ struct nbd_device {
>  
>   struct mutex tx_lock;
>   struct gendisk *disk;
> - int blksize;
> + loff_t blksize;
>   loff_t bytesize;
>   int xmit_timeout;
>   bool disconnect; /* a disconnect has been requested by user */
> @@ -135,7 +135,7 @@ static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
>  }
>  
>  static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
> - int blocksize, int nr_blocks)
> + loff_t blocksize, loff_t nr_blocks)
>  {
>   int ret;
>  
> @@ -144,7 +144,7 @@ static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
>   return ret;
>  
>   nbd->blksize = blocksize;
> - nbd->bytesize = (loff_t)blocksize * (loff_t)nr_blocks;
> + nbd->bytesize = blocksize * nr_blocks;
>  
>   nbd_size_update(nbd, bdev);
>  
> @@ -959,7 +959,7 @@ static int nbd_dev_dbg_init(struct nbd_device *nbd)
>   return PTR_ERR(f);
>   }
>  
> - f = debugfs_create_u32("blocksize", 0444, dir, &nbd->blksize);
> + f = debugfs_create_u64("blocksize", 0444, dir, &nbd->blksize);
>   if (IS_ERR_OR_NULL(f)) {
>   dev_err(nbd_to_dev(nbd), "Failed to create debugfs file 'blocksize', %ld\n",
>   PTR_ERR(f));
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH] nbd: fix 64-bit division

Juerg Haefliger
In reply to this post by Joseph Salisbury-3
From: Jens Axboe <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1772575

We have this:

ERROR: "__aeabi_ldivmod" [drivers/block/nbd.ko] undefined!
ERROR: "__divdi3" [drivers/block/nbd.ko] undefined!
nbd.c:(.text+0x247c72): undefined reference to `__divdi3'

due to a recent commit, that did 64-bit division. Use the proper
divider function so that 32-bit compiles don't break.

Fixes: ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")
Signed-off-by: Jens Axboe <[hidden email]>
(cherry picked from commit e88f72cb9f54f6d244e55f629fe5e2f34ca6f9ed)
Signed-off-by: Juerg Haefliger <[hidden email]>
---
 drivers/block/nbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 673632c301dc..754b6fd9d2f4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -750,7 +750,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 
  case NBD_SET_SIZE:
  return nbd_size_set(nbd, bdev, nbd->blksize,
-    arg / nbd->blksize);
+ div_s64(arg, nbd->blksize));
 
  case NBD_SET_SIZE_BLOCKS:
  return nbd_size_set(nbd, bdev, nbd->blksize, arg);
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Xenial][PATCH] nbd: fix 64-bit division

Juerg Haefliger
We also need this patch to prevent compilation errors on non-64bit arches.

...Juerg

On 06/06/2018 02:22 PM, Juerg Haefliger wrote:

> From: Jens Axboe <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> We have this:
>
> ERROR: "__aeabi_ldivmod" [drivers/block/nbd.ko] undefined!
> ERROR: "__divdi3" [drivers/block/nbd.ko] undefined!
> nbd.c:(.text+0x247c72): undefined reference to `__divdi3'
>
> due to a recent commit, that did 64-bit division. Use the proper
> divider function so that 32-bit compiles don't break.
>
> Fixes: ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit e88f72cb9f54f6d244e55f629fe5e2f34ca6f9ed)
> Signed-off-by: Juerg Haefliger <[hidden email]>
> ---
>  drivers/block/nbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 673632c301dc..754b6fd9d2f4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -750,7 +750,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  
>   case NBD_SET_SIZE:
>   return nbd_size_set(nbd, bdev, nbd->blksize,
> -    arg / nbd->blksize);
> + div_s64(arg, nbd->blksize));
>  
>   case NBD_SET_SIZE_BLOCKS:
>   return nbd_size_set(nbd, bdev, nbd->blksize, arg);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH] nbd: fix 64-bit division

Thadeu Lima de Souza Cascardo-3
In reply to this post by Juerg Haefliger
On Wed, Jun 06, 2018 at 02:22:23PM +0200, Juerg Haefliger wrote:

> From: Jens Axboe <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> We have this:
>
> ERROR: "__aeabi_ldivmod" [drivers/block/nbd.ko] undefined!
> ERROR: "__divdi3" [drivers/block/nbd.ko] undefined!
> nbd.c:(.text+0x247c72): undefined reference to `__divdi3'
>
> due to a recent commit, that did 64-bit division. Use the proper
> divider function so that 32-bit compiles don't break.
>
> Fixes: ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit e88f72cb9f54f6d244e55f629fe5e2f34ca6f9ed)
> Signed-off-by: Juerg Haefliger <[hidden email]>
> ---
>  drivers/block/nbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 673632c301dc..754b6fd9d2f4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -750,7 +750,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  
>   case NBD_SET_SIZE:
>   return nbd_size_set(nbd, bdev, nbd->blksize,
> -    arg / nbd->blksize);
> + div_s64(arg, nbd->blksize));
>  
>   case NBD_SET_SIZE_BLOCKS:
>   return nbd_size_set(nbd, bdev, nbd->blksize, arg);
> --
> 2.17.1

Acked-by: Thadeu Lima de Souza Cascardo <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH] nbd: fix 64-bit division

Stefan Bader-2
In reply to this post by Juerg Haefliger
On 06.06.2018 05:22, Juerg Haefliger wrote:

> From: Jens Axboe <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> We have this:
>
> ERROR: "__aeabi_ldivmod" [drivers/block/nbd.ko] undefined!
> ERROR: "__divdi3" [drivers/block/nbd.ko] undefined!
> nbd.c:(.text+0x247c72): undefined reference to `__divdi3'
>
> due to a recent commit, that did 64-bit division. Use the proper
> divider function so that 32-bit compiles don't break.
>
> Fixes: ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit e88f72cb9f54f6d244e55f629fe5e2f34ca6f9ed)
> Signed-off-by: Juerg Haefliger <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>

> ---
>  drivers/block/nbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 673632c301dc..754b6fd9d2f4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -750,7 +750,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  
>   case NBD_SET_SIZE:
>   return nbd_size_set(nbd, bdev, nbd->blksize,
> -    arg / nbd->blksize);
> + div_s64(arg, nbd->blksize));
>  
>   case NBD_SET_SIZE_BLOCKS:
>   return nbd_size_set(nbd, bdev, nbd->blksize, arg);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED: [SRU][Xenial][PATCH] nbd: fix 64-bit division

Juerg Haefliger
In reply to this post by Juerg Haefliger
Applied to xenial/master-next.

...Juerg

On 06/06/2018 02:22 PM, Juerg Haefliger wrote:

> From: Jens Axboe <[hidden email]>
>
> BugLink: http://bugs.launchpad.net/bugs/1772575
>
> We have this:
>
> ERROR: "__aeabi_ldivmod" [drivers/block/nbd.ko] undefined!
> ERROR: "__divdi3" [drivers/block/nbd.ko] undefined!
> nbd.c:(.text+0x247c72): undefined reference to `__divdi3'
>
> due to a recent commit, that did 64-bit division. Use the proper
> divider function so that 32-bit compiles don't break.
>
> Fixes: ef77b515243b ("nbd: use loff_t for blocksize and nbd_set_size args")
> Signed-off-by: Jens Axboe <[hidden email]>
> (cherry picked from commit e88f72cb9f54f6d244e55f629fe5e2f34ca6f9ed)
> Signed-off-by: Juerg Haefliger <[hidden email]>
> ---
>  drivers/block/nbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 673632c301dc..754b6fd9d2f4 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -750,7 +750,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
>  
>   case NBD_SET_SIZE:
>   return nbd_size_set(nbd, bdev, nbd->blksize,
> -    arg / nbd->blksize);
> + div_s64(arg, nbd->blksize));
>  
>   case NBD_SET_SIZE_BLOCKS:
>   return nbd_size_set(nbd, bdev, nbd->blksize, arg);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment