public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Bae, Chang Seok" <chang.seok.bae@intel.com>
To: Borislav Petkov <bp@suse.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"luto@kernel.org" <luto@kernel.org>,
	"x86@kernel.org" <x86@kernel.org>,
	"Brown, Len" <len.brown@intel.com>,
	"Hansen, Dave" <dave.hansen@intel.com>,
	"hjl.tools@gmail.com" <hjl.tools@gmail.com>,
	"Dave.Martin@arm.com" <Dave.Martin@arm.com>,
	"jannh@google.com" <jannh@google.com>,
	"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
	"carlos@redhat.com" <carlos@redhat.com>,
	"Luck, Tony" <tony.luck@intel.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	"libc-alpha@sourceware.org" <libc-alpha@sourceware.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow
Date: Thu, 25 Mar 2021 17:21:04 +0000	[thread overview]
Message-ID: <06722BDE-738A-4513-886E-2C1442C97369@intel.com> (raw)
In-Reply-To: <20210325162047.GA32296@zn.tnic>

On Mar 25, 2021, at 09:20, Borislav Petkov <bp@suse.de> wrote:
> 
> $ gcc tst-minsigstksz-2.c -DMY_MINSIGSTKSZ=3453 -o tst-minsigstksz-2
> $ ./tst-minsigstksz-2
> tst-minsigstksz-2: changed byte 50 bytes below configured stack
> 
> Whoops.
> 
> And the debug print said:
> 
> [ 5395.252884] signal: get_sigframe: sp: 0x7f54ec39e7b8, sas_ss_sp: 0x7f54ec39e6ce, sas_ss_size 0xd7d
> 
> which tells me that, AFAICT, your check whether we have enough alt stack
> doesn't seem to work in this case.

Yes, in this case.

tst-minsigstksz-2.c has this code:

static void
handler (int signo)
{
  /* Clear a bit of on-stack memory.  */
  volatile char buffer[256];
  for (size_t i = 0; i < sizeof (buffer); ++i)
    buffer[i] = 0;
  handler_run = 1;
}
…

  if (handler_run != 1)
    errx (1, "handler did not run");

  for (void *p = stack_buffer; p < stack_bottom; ++p)
    if (*(unsigned char *) p != 0xCC)
      errx (1, "changed byte %zd bytes below configured stack\n",
            stack_bottom - p);
…

I think the message comes from the handler’s overwriting, not from the kernel.

The patch's check is to detect and prevent the kernel-induced overflow --
whether alt stack enough for signal delivery itself.  The stack is possibly
not enough for the signal handler's use as the kernel does not know for it.

Thanks,
Chang






  reply	other threads:[~2021-03-25 17:21 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-16  6:52 [PATCH v7 0/6] x86: Improve Minimum Alternate Stack Size Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 1/6] uapi: Define the aux vector AT_MINSIGSTKSZ Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 2/6] x86/signal: Introduce helpers to get the maximum signal frame size Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 3/6] x86/elf: Support a new ELF aux vector AT_MINSIGSTKSZ Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 4/6] selftest/sigaltstack: Use the AT_MINSIGSTKSZ aux vector if available Chang S. Bae
2021-03-16  6:52 ` [PATCH v7 5/6] x86/signal: Detect and prevent an alternate signal stack overflow Chang S. Bae
2021-03-16 11:52   ` Borislav Petkov
2021-03-16 18:26     ` Bae, Chang Seok
2021-03-25 16:20       ` Borislav Petkov
2021-03-25 17:21         ` Bae, Chang Seok [this message]
2021-03-25 20:14           ` Florian Weimer
2021-03-25 18:13   ` Andy Lutomirski
2021-03-25 18:54     ` Borislav Petkov
2021-03-25 21:11       ` Bae, Chang Seok
2021-03-25 21:27         ` Borislav Petkov
2021-03-26  4:56       ` Andy Lutomirski
2021-03-26 10:30         ` Borislav Petkov
2021-04-12 22:30           ` Bae, Chang Seok
2021-04-14 10:12             ` Borislav Petkov
2021-04-14 11:30               ` Florian Weimer
2021-04-14 12:06                 ` Borislav Petkov
2021-05-03  5:30                   ` Florian Weimer
2021-05-03 11:17                     ` Borislav Petkov
2021-03-26  4:58     ` Andy Lutomirski
2021-03-16  6:52 ` [PATCH v7 6/6] selftest/x86/signal: Include test cases for validating sigaltstack Chang S. Bae
2021-03-17 10:06 ` [PATCH v7 0/6] x86: Improve Minimum Alternate Stack Size Ingo Molnar
2021-03-17 10:44   ` Ingo Molnar
2021-03-19 18:12     ` Len Brown
2021-03-20 17:32       ` Ingo Molnar

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=06722BDE-738A-4513-886E-2C1442C97369@intel.com \
    --to=chang.seok.bae@intel.com \
    --cc=Dave.Martin@arm.com \
    --cc=bp@suse.de \
    --cc=carlos@redhat.com \
    --cc=dave.hansen@intel.com \
    --cc=hjl.tools@gmail.com \
    --cc=jannh@google.com \
    --cc=len.brown@intel.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=ravi.v.shankar@intel.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).