Re: [PATCH] Fix sis driver to compile with -Werror=format-security
> It's a string literal. How am I trying to confuse the tools? It would > change the argument to actually match what's expected (string literal > for the format field). Thats a bug in the tools. Its perfectly valid to pass a non string literal to those functions providing the contents are correct. Alan ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
'Twas brillig, and Ander Conselvan de Oliveira at 23/12/08 18:10 did gyre and gimble: > Em Tuesday 23 December 2008 15:43:26 Eric Anholt escreveu: >> On Tue, 2008-12-23 at 13:25 -0200, Ander Conselvan de Oliveira wrote: >>> The attached patch fix the sis driver to compile with >>> -Werror=format-security. >> Some of those strings are predefined and visible in the patch you posted >> (not a security issue). I certainly wouldn't apply this patch. > > To be honest, I was not concerned with the security issues this might have > but > with the fact that is does not compile. Mandriva's build system sets this > flag by default and this might be the case for other distros. Well we are doing it based on inspiration from other distros: http://wiki.mandriva.com/en/Development/Packaging/Problems#format_not_a_string_literal_and_no_format_arguments http://wiki.debian.org/Hardening https://wiki.ubuntu.com/CompilerFlags So I think it would be beneficial to get this fix into the official repos. I posted a similar fix for xserver a few days back which ajax applied. While the "fix" is arguably unnecessary, they are also trivial so I wouldn't have thought they would be overly controversial, considering the potential issues that could be caught. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mandriva Linux Contributor [http://www.mandriva.com/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/] ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
On Tue, Dec 23, 2008 at 10:22 AM, Alan Cox wrote: >> > better patch. On the other hand, if you guys feel there is nothing to be >> > fixed here I'll just keep the patch for the Mandriva package and let it go. >> >> Change str to a macro and use that: >> >> #define MSG_SEP "**\n" >> ... >> xf86DrvMsg(pScrn->scrnIndex, X_ERROR, MSG_SEP); >> ... >> #undef MSG_STR > > GAK > > Thats horrible on so many levels > > 1. It uglifiers the code enormously Why is defining a temporary macro any worse than creating a global character array? It's not like this is going in a public header or something. People create temporary convienience macros all the time. > 2. You are trying to hide a compiler thing by confusing the tools, and > the tools will get smarter and work that out It's a string literal. How am I trying to confuse the tools? It would change the argument to actually match what's expected (string literal for the format field). If it's really that bad, then I rescind my advice and will move on with my life. :) -- Dan ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
> > better patch. On the other hand, if you guys feel there is nothing to be > > fixed here I'll just keep the patch for the Mandriva package and let it go. > > Change str to a macro and use that: > > #define MSG_SEP "**\n" > ... > xf86DrvMsg(pScrn->scrnIndex, X_ERROR, MSG_SEP); > ... > #undef MSG_STR GAK Thats horrible on so many levels 1. It uglifiers the code enormously 2. You are trying to hide a compiler thing by confusing the tools, and the tools will get smarter and work that out Alan ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
On Tue, Dec 23, 2008 at 10:10 AM, Ander Conselvan de Oliveira wrote: > Em Tuesday 23 December 2008 15:43:26 Eric Anholt escreveu: >> On Tue, 2008-12-23 at 13:25 -0200, Ander Conselvan de Oliveira wrote: >> > The attached patch fix the sis driver to compile with >> > -Werror=format-security. >> >> Some of those strings are predefined and visible in the patch you posted >> (not a security issue). I certainly wouldn't apply this patch. > > To be honest, I was not concerned with the security issues this might have but > with the fact that is does not compile. Mandriva's build system sets this > flag by default and this might be the case for other distros. > > If there is a better fix for the compilation issue I'll be happy to send a > better patch. On the other hand, if you guys feel there is nothing to be > fixed here I'll just keep the patch for the Mandriva package and let it go. Change str to a macro and use that: #define MSG_SEP "**\n" ... xf86DrvMsg(pScrn->scrnIndex, X_ERROR, MSG_SEP); ... #undef MSG_STR -- Dan ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
Em Tuesday 23 December 2008 15:43:26 Eric Anholt escreveu: > On Tue, 2008-12-23 at 13:25 -0200, Ander Conselvan de Oliveira wrote: > > The attached patch fix the sis driver to compile with > > -Werror=format-security. > > Some of those strings are predefined and visible in the patch you posted > (not a security issue). I certainly wouldn't apply this patch. To be honest, I was not concerned with the security issues this might have but with the fact that is does not compile. Mandriva's build system sets this flag by default and this might be the case for other distros. If there is a better fix for the compilation issue I'll be happy to send a better patch. On the other hand, if you guys feel there is nothing to be fixed here I'll just keep the patch for the Mandriva package and let it go. Thanks, Ander ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
Re: [PATCH] Fix sis driver to compile with -Werror=format-security
On Tue, 2008-12-23 at 13:25 -0200, Ander Conselvan de Oliveira wrote: > The attached patch fix the sis driver to compile with -Werror=format-security. Some of those strings are predefined and visible in the patch you posted (not a security issue). I certainly wouldn't apply this patch. -- Eric Anholt e...@anholt.net eric.anh...@intel.com signature.asc Description: This is a digitally signed message part ___ xorg mailing list xorg@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/xorg
