Re: [PATCH] libmultipath: use asprintf() to allocate prefixed_uuid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2022-01-25 at 23:35 -0600, Benjamin Marzinski wrote:
> gcc 12.0.1 failed building libmultipath due to a format-overflow
> false
> positive on 32-bit architectures.  This isn't so surprising as
> format-overflow=2 is very aggressive in the assumptions it makes
> about
> the arguments.  Here, it assumes that mpp->wwid could take up all the
> space that a pointer could point to, even if I add code to this
> function
> to explicitly null terminate mpp->wwid to fit in WWID_SIZE.

This sounds like a bug in gcc which should be reported.

> 
> To avoid this and simplify the function, switch from using calloc()
> and
> sprintf() to just using asprintf().
> 
> For reference, the gcc build error that this fixes is:
> 
> devmapper.c: In function 'dm_addmap.constprop.0':
> devmapper.h:27:21: error: '%s' directive writing up to 2147483644
> bytes into a region of size 2147483641 [-Werror=format-overflow=]
>    27 | #define UUID_PREFIX "mpath-"
>       |                     ^~~~~~~~
> devmapper.c:484:53: note: format string is defined here
>   484 |                 sprintf(prefixed_uuid, UUID_PREFIX "%s", mpp-
> >wwid);
>       |                                                     ^~
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>

Reviewed-by: Martin Wilck <mwilck@xxxxxxxx>

> ---
>  libmultipath/devmapper.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 36038150..2507f77f 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -473,14 +473,11 @@ dm_addmap (int task, const char *target, struct
> multipath *mpp,
>                 dm_task_set_ro(dmt);
>  
>         if (task == DM_DEVICE_CREATE) {
> -               prefixed_uuid = calloc(1, UUID_PREFIX_LEN +
> -                                      strlen(mpp->wwid) + 1);
> -               if (!prefixed_uuid) {
> +               if (asprintf(&prefixed_uuid, UUID_PREFIX "%s", mpp-
> >wwid) < 0) {
>                         condlog(0, "cannot create prefixed uuid :
> %s",
>                                 strerror(errno));
>                         goto addout;
>                 }
> -               sprintf(prefixed_uuid, UUID_PREFIX "%s", mpp->wwid);
>                 if (!dm_task_set_uuid(dmt, prefixed_uuid))
>                         goto freeout;
>                 dm_task_skip_lockfs(dmt);


--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux