For reasons unknown, when rewriting this code and dropping
libdevmapper I've mistakenly used incorrect length of dm.name. In
linux/dm-ioctl.h the dm_ioctl struct is defined as follows:

  #define DM_NAME_LEN 128

  struct dm_ioctl {
    ...
    char name[DM_NAME_LEN];     /* device name */
    ...
  };

However, when copying string into this member, DM_TABLE_DEPS was
used, which is defined as follows:

  #define DM_TABLE_DEPS    _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)

After decryption, this results in the following size: 3241737483.

Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virdevmapper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index fcb11e954f..2c4c2df999 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -240,7 +240,7 @@ virDevMapperGetTargetsImpl(int controlFD,
     if (!(sanitizedPath = virDMSanitizepath(path)))
         return 0;
 
-    if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) < 0) {
+    if (virStrcpy(dm.name, sanitizedPath, DM_NAME_LEN) < 0) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("Resolved device mapper name too long"));
         return -1;
-- 
2.26.2

On Mon, Mar 08, 2021 at 09:14:18AM +0100, Michal Privoznik wrote:
> For reasons unknown, when rewriting this code and dropping
> libdevmapper I've mistakenly used incorrect length of dm.name. In
> linux/dm-ioctl.h the dm_ioctl struct is defined as follows:
> 
>   #define DM_NAME_LEN 128
> 
>   struct dm_ioctl {
>     ...
>     char name[DM_NAME_LEN];     /* device name */
>     ...
>   };
> 
> However, when copying string into this member, DM_TABLE_DEPS was
> used, which is defined as follows:
> 
>   #define DM_TABLE_DEPS    _IOWR(DM_IOCTL, DM_TABLE_DEPS_CMD, struct dm_ioctl)
> 
> After decryption, this results in the following size: 3241737483.
> 
> Fixes: 22494556542c676d1b9e7f1c1f2ea13ac17e1e3e
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/virdevmapper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

So we were not correctly capping the input path length. Bad
but IIUC not a security bug  because the input is controlled by
a client who already has privileges equivalent to root.

> 
> diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
> index fcb11e954f..2c4c2df999 100644
> --- a/src/util/virdevmapper.c
> +++ b/src/util/virdevmapper.c
> @@ -240,7 +240,7 @@ virDevMapperGetTargetsImpl(int controlFD,
>      if (!(sanitizedPath = virDMSanitizepath(path)))
>          return 0;
>  
> -    if (virStrcpy(dm.name, sanitizedPath, DM_TABLE_DEPS) < 0) {
> +    if (virStrcpy(dm.name, sanitizedPath, DM_NAME_LEN) < 0) {
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>                         _("Resolved device mapper name too long"));
>          return -1;

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|