All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Julien Grall <julien@xen.org>
Cc: "Julien Grall" <jgrall@amazon.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Dario Faggioli" <dfaggioli@suse.com>,
	"Tim Deegan" <tim@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 00/14] Use const whether we point to literal strings (take 1)
Date: Tue, 6 Apr 2021 09:50:44 +0200	[thread overview]
Message-ID: <bdcf123c-0345-a496-03fa-4279726469ea@suse.com> (raw)
In-Reply-To: <20210405155713.29754-1-julien@xen.org>

On 05.04.2021 17:56, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Hi all,
> 
> By default, both Clang and GCC will happily compile C code where
> non-const char * point to literal strings. This means the following
> code will be accepted:
> 
>     char *str = "test";
> 
>     str[0] = 'a';
> 
> Literal strings will reside in rodata, so they are not modifiable.
> This will result to an permission fault at runtime if the permissions
> are enforced in the page-tables (this is the case in Xen).
> 
> I am not aware of code trying to modify literal strings in Xen.
> However, there is a frequent use of non-const char * to point to
> literal strings. Given the size of the codebase, there is a risk
> to involuntarily introduce code that will modify literal strings.
> 
> Therefore it would be better to enforce using const when pointing
> to such strings. Both GCC and Clang provide an option to warn
> for such case (see -Wwrite-strings) and therefore could be used
> by Xen.
> 
> This series doesn't yet make use of -Wwrite-strings because
> the tree is not fully converted. Instead, it contains some easy
> and likely non-controversial use const in the code.
> 
> The major blockers to enable -Wwrite-strings are the following:
>     - xen/common/efi: union string is used in both const and
>     non-const situation. It doesn't feel right to specific one member
>     const and the other non-const.

I'd be happy to see a suggestion of how to avoid this in a not overly
intrusive way.

>     - libxl: the major block is the flexarray framework as we would use
>     it with string (now const char*). I thought it would be possible to
>     make the interface const, but it looks like there are a couple of
>     places where we need to modify the content (such as in
>     libxl_json.c).
> 
> Ideally, I would like to have -Wwrite-strings unconditionally used
> tree-wide. But, some of the area may required some heavy refactoring.
> 
> One solution would be to enable it tree-wide but turned it off at a
> directroy/file level.

At least as a transient approach I think this would make sense. EFI in
particular has other reasons already to specify a custom option
(-fshort-wchar).

Jan


  parent reply	other threads:[~2021-04-06  7:50 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 15:56 [PATCH 00/14] Use const whether we point to literal strings (take 1) Julien Grall
2021-04-05 15:57 ` [PATCH 01/14] xen: Constify the second parameter of rangeset_new() Julien Grall
2021-04-06  7:57   ` Jan Beulich
2021-04-06 18:03     ` Julien Grall
2021-04-05 15:57 ` [PATCH 02/14] xen/sched: Constify name and opt_name in struct scheduler Julien Grall
2021-04-06  8:07   ` Jan Beulich
2021-04-06 18:24     ` Julien Grall
2021-04-07  8:22       ` Jan Beulich
2021-04-07  9:06         ` Julien Grall
2021-04-06 14:19   ` George Dunlap
2021-04-05 15:57 ` [PATCH 03/14] xen/x86: shadow: The return type of sh_audit_flags() should be const Julien Grall
2021-04-06  7:24   ` Roger Pau Monné
2021-04-06 18:26     ` Julien Grall
2021-04-06 14:00   ` Tim Deegan
2021-04-05 15:57 ` [PATCH 04/14] xen/char: console: Use const whenever we point to literal strings Julien Grall
2021-04-06  8:10   ` Jan Beulich
2021-04-06 18:27     ` Julien Grall
2021-04-05 15:57 ` [PATCH 05/14] tools/libs: guest: " Julien Grall
2021-05-11 14:58   ` Anthony PERARD
2021-05-18 13:33     ` Julien Grall
2021-04-05 15:57 ` [PATCH 06/14] tools/libs: stat: " Julien Grall
2021-05-11 15:03   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 07/14] tools/xl: " Julien Grall
2021-04-27 16:04   ` Anthony PERARD
2021-04-27 16:28     ` Julien Grall
2021-04-27 17:03       ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 08/14] tools/firmware: hvmloader: Use const in __bug() and __assert_failed() Julien Grall
2021-04-06  7:29   ` Roger Pau Monné
2021-04-06 19:02     ` Julien Grall
2021-04-05 15:57 ` [PATCH 09/14] tools/console: Use const whenever we point to literal strings Julien Grall
2021-05-11 15:18   ` Anthony PERARD
2021-05-18 13:48     ` Julien Grall
2021-04-05 15:57 ` [PATCH 10/14] tools/kdd: " Julien Grall
2021-04-06 14:03   ` Tim Deegan
2021-04-05 15:57 ` [PATCH 11/14] tools/misc: " Julien Grall
2021-05-11 15:37   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 12/14] tools/top: The string parameter in set_prompt() and set_delay() should be const Julien Grall
2021-05-11 15:46   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 13/14] tools/xenmon: xenbaked: Mark const the field text in stat_map_t Julien Grall
2021-05-11 16:08   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 14/14] tools/xentrace: Use const whenever we point to literal strings Julien Grall
2021-04-06 14:15   ` George Dunlap
2021-04-05 17:01 ` [PATCH 00/14] Use const whether we point to literal strings (take 1) Elliott Mitchell
2021-04-06 17:55   ` Julien Grall
2021-04-06  7:50 ` Jan Beulich [this message]
2021-04-06 19:08 ` Julien Grall
2021-05-10 17:49 ` PING " Julien Grall
2021-05-17 18:41   ` Wei Liu
2021-05-18 14:02     ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bdcf123c-0345-a496-03fa-4279726469ea@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=dfaggioli@suse.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jgrall@amazon.com \
    --cc=julien@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.