All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Kees Cook <keescook@chromium.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Jinbum Park <jinb.park7@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH] mm: fix RODATA_TEST failure "rodata_test: test data was not read only"
Date: Mon, 25 Sep 2017 02:37:21 -0500	[thread overview]
Message-ID: <20170925073721.GM8421@gate.crashing.org> (raw)
In-Reply-To: <CAGXu5jJ54+bCcXaPK1ExsxtTDPHNn1+1gywb3TDbe-SEtt1zuQ@mail.gmail.com>

On Sun, Sep 24, 2017 at 12:17:51PM -0700, Kees Cook wrote:
> On Thu, Sep 21, 2017 at 2:37 AM, Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
> > On powerpc, RODATA_TEST fails with message the following messages:
> >
> > [    6.199505] Freeing unused kernel memory: 528K
> > [    6.203935] rodata_test: test data was not read only
> >
> > This is because GCC allocates it to .data section:
> >
> > c0695034 g     O .data  00000004 rodata_test_data
> 
> Uuuh... that seems like a compiler bug. It's marked "const" -- it
> should never end up in .data. I would argue that this has done exactly
> what it was supposed to do, and shows that something has gone wrong.
> It should always be const. Adding "static" should just change
> visibility. (I'm not opposed to the static change, but it seems to
> paper over a problem with the compiler...)

The compiler puts this item in .sdata, for 32-bit.  There is no .srodata,
so if it wants to use a small data section, it must use .sdata .

Non-external, non-referenced symbols are not put in .sdata, that is the
difference you see with the "static".

I don't think there is a bug here.  If you think there is, please open
a GCC bug.


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Kees Cook <keescook@chromium.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Jinbum Park <jinb.park7@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH] mm: fix RODATA_TEST failure "rodata_test: test data was not read only"
Date: Mon, 25 Sep 2017 02:37:21 -0500	[thread overview]
Message-ID: <20170925073721.GM8421@gate.crashing.org> (raw)
In-Reply-To: <CAGXu5jJ54+bCcXaPK1ExsxtTDPHNn1+1gywb3TDbe-SEtt1zuQ@mail.gmail.com>

On Sun, Sep 24, 2017 at 12:17:51PM -0700, Kees Cook wrote:
> On Thu, Sep 21, 2017 at 2:37 AM, Christophe Leroy
> <christophe.leroy@c-s.fr> wrote:
> > On powerpc, RODATA_TEST fails with message the following messages:
> >
> > [    6.199505] Freeing unused kernel memory: 528K
> > [    6.203935] rodata_test: test data was not read only
> >
> > This is because GCC allocates it to .data section:
> >
> > c0695034 g     O .data  00000004 rodata_test_data
> 
> Uuuh... that seems like a compiler bug. It's marked "const" -- it
> should never end up in .data. I would argue that this has done exactly
> what it was supposed to do, and shows that something has gone wrong.
> It should always be const. Adding "static" should just change
> visibility. (I'm not opposed to the static change, but it seems to
> paper over a problem with the compiler...)

The compiler puts this item in .sdata, for 32-bit.  There is no .srodata,
so if it wants to use a small data section, it must use .sdata .

Non-external, non-referenced symbols are not put in .sdata, that is the
difference you see with the "static".

I don't think there is a bug here.  If you think there is, please open
a GCC bug.


Segher

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-09-25  7:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-21  9:37 [PATCH] mm: fix RODATA_TEST failure "rodata_test: test data was not read only" Christophe Leroy
2017-09-21  9:37 ` Christophe Leroy
2017-09-24 19:17 ` Kees Cook
2017-09-24 19:17   ` Kees Cook
2017-09-25  7:37   ` Segher Boessenkool [this message]
2017-09-25  7:37     ` Segher Boessenkool
2017-09-25 16:01     ` David Laight
2017-09-25 16:01       ` David Laight
2017-09-25 16:01       ` David Laight
2017-09-25 19:41       ` Segher Boessenkool
2017-09-25 19:41         ` Segher Boessenkool
2017-10-02 19:29         ` Kees Cook
2017-10-02 19:29           ` Kees Cook
2017-10-02 20:08           ` Segher Boessenkool
2017-10-02 20:08             ` Segher Boessenkool
2017-10-02 20:27             ` Kees Cook
2017-10-02 20:27               ` Kees Cook

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=20170925073721.GM8421@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=akpm@linux-foundation.org \
    --cc=christophe.leroy@c-s.fr \
    --cc=jinb.park7@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    /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.