[vlc-devel] [PATCH] control: rc: avoid stringop-truncation warning

[Alexandre Janniaux]

Hi,

On Mon, Jan 20, 2020 at 05:27:11PM +0200, Rémi Denis-Courmont wrote:
> Hi,
>
> Truncating a file path silently is wrong. I thought that's obvious TBH.
>

Sorry to not match your expectation, but thank you for the notice.
I'll provide a fix for this then.

Regards,
--
Alexandre Janniaux
Videolabs

> Le 20 janvier 2020 17:22:18 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
> >Unfortunately, I'm not sure I understand what problem you're talking
> >about. Can you be more explicit about it?
> >
> >AFAIU in the current situation, there was no issues but only an unsafe
> >usage of strncpy which was not harmful as it was protected by an ending
> >'\0' character enforcement. I don't see what the warning is
> >highlighting otherwise.
> >
> >Regards,
> >--
> >Alexandre Janniaux
> >Videolabs
> >
> >On Mon, Jan 20, 2020 at 04:44:32PM +0200, Rémi Denis-Courmont wrote:
> >> Hi,
> >>
> >> The point is that the warning points to a real problem with the
> >implementation, which the patch does not fix. And as a general rule,
> >I'm against removing warnings without fixing the real problem.
> >>
> >> Le 20 janvier 2020 15:51:03 GMT+02:00, Alexandre Janniaux
> ><ajanni at videolabs.io> a écrit :
> >> >Hi,
> >> >
> >> >There should be no functional differences as the null character is
> >> >forced in the end of the path. Copying one meaningful character less
> >> >would happend as soon as the path is already too long, and otherwise
> >> >only the strncpy null character is expected to be truncated, but is
> >> >ensured by the code following.
> >> >
> >> >The documentation state that sun_path should contain a
> >null-terminated
> >> >pathname so it doesn't look wrong to enforce the last \0 byte too.
> >Am
> >> >I missing something?
> >> >
> >> >I hope I'm clear enough,
> >> >
> >> >Regards,
> >> >--
> >> >Alexandre Janniaux
> >> >Videolabs
> >> >
> >> >On Mon, Jan 20, 2020 at 03:29:43PM +0200, Rémi Denis-Courmont wrote:
> >> >> On second consideration, chopping the path is not correct
> >behaviour.
> >> >This patch is concealing the bug highlighted by the warning rather
> >than
> >> >fixing it IMO.
> >> >>
> >> >> Better drop the feature altogether as I dot think anyone's using
> >it.
> >> >>
> >> >> Le 20 janvier 2020 09:38:46 GMT+02:00, "Rémi Denis-Courmont"
> >> ><remi at remlab.net> a écrit :
> >> >> >OK, though I'd suggest removing Unix socket support entirely as
> >the
> >> >old
> >> >> >RC "protocol" is not suitable for programmatic usage.
> >> >> >
> >> >> >Le 19 janvier 2020 17:39:27 GMT+02:00, Alexandre Janniaux
> >> >> ><ajanni at videolabs.io> a écrit :
> >> >> >>Fortify enabled implies a warning if we use strncpy with a
> >length
> >> >> >equal
> >> >> >>to the buffer size. As we fill the last item with a null
> >character,
> >> >we
> >> >> >>can use length-1 instead as well.
> >> >> >>---
> >> >> >> modules/control/rc.c | 2 +-
> >> >> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >> >>
> >> >> >>diff --git a/modules/control/rc.c b/modules/control/rc.c
> >> >> >>index cd85602d59..dc37d352ad 100644
> >> >> >>--- a/modules/control/rc.c
> >> >> >>+++ b/modules/control/rc.c
> >> >> >>@@ -1730,7 +1730,7 @@ static int Activate( vlc_object_t *p_this
> >)
> >> >> >>         }
> >> >> >>
> >> >> >>         addr.sun_family = AF_LOCAL;
> >> >> >>-        strncpy( addr.sun_path, psz_unix_path, sizeof(
> >> >addr.sun_path
> >> >> >)
> >> >> >>);
> >> >> >>+        strncpy( addr.sun_path, psz_unix_path, sizeof(
> >> >addr.sun_path
> >> >> >)
> >> >> >>- 1 );
> >> >> >>         addr.sun_path[sizeof( addr.sun_path ) - 1] = '\0';
> >> >> >>
> >> >> >>         if (bind (i_socket, (struct sockaddr *)&addr, sizeof
> >> >(addr))
> >> >> >>--
> >> >> >>2.25.0
> >> >> >>
> >> >> >>_______________________________________________
> >> >> >>vlc-devel mailing list
> >> >> >>To unsubscribe or modify your subscription options:
> >> >> >>https://mailman.videolan.org/listinfo/vlc-devel
> >> >> >
> >> >> >--
> >> >> >Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >> >excuser
> >> >> >ma brièveté.
> >> >>
> >> >> --
> >> >> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >> >excuser ma brièveté.
> >> >
> >> >> _______________________________________________
> >> >> vlc-devel mailing list
> >> >> To unsubscribe or modify your subscription options:
> >> >> https://mailman.videolan.org/listinfo/vlc-devel
> >> >_______________________________________________
> >> >vlc-devel mailing list
> >> >To unsubscribe or modify your subscription options:
> >> >https://mailman.videolan.org/listinfo/vlc-devel
> >>
> >> --
> >> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> >excuser ma brièveté.
> >
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel