[PATCH] virDevMapperGetTargetsImpl: Use correct length when copying into dm.name

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 :|
participants (2)
-
Daniel P. Berrangé
-
Michal Privoznik