Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 4 Jun 2021 15:06:59 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     Jiashuo Liang <liangjs@pku.edu.cn>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] signal/x86: Don't send SIGSEGV twice on SEGV_PKUERR
Message-ID: <YLolc7z64h9yHNao@zn.tnic>
References: <20210601085203.40214-1-liangjs@pku.edu.cn>
 <YLkhV+lSqXlcfUc5@zn.tnic>
 <87lf7qocsd.fsf@disp2133>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <87lf7qocsd.fsf@disp2133>
Precedence: bulk

On Thu, Jun 03, 2021 at 04:31:46PM -0500, Eric W. Biederman wrote:
> There are two ways signals get delivered.  The old fashioned way in the
> signal bitmap, and the new fangled way by queuing sigqueue_info.

By that you mean that third arg siginfo_t to

SYSCALL_DEFINE3(rt_sigqueueinfo, pid_t, pid, int, sig,
                siginfo_t __user *, uinfo)

I presume?

Which, as sigqueue(3) says, is what is called on Linux.

> In the old fashioned way there is no information except that the
> signal itself was delivered, and if the signal is sent twice it
> is impossible to find out. In the new fangled way because the
> sigqueue_info can vary between different times a signal is sent you
> can both see that a signal was delivered twice (because there are two
> distinct entries in the queue), but also possibly tell those two times
> a signal was sent apart.
>
> The new real time signals can queue as many sigqueue_info's as their
> rlimit allows.  The old signals are limited to exactly one sigqueue_info
> per signal number.

Aha.

> In this case the legacy_queue check tests to see if the signal is
> already pending (present in the signal bitmap) and not a new real time
> signal (which means only one sigqueue_info entry is allowed in the
> signal queue).

Aha, that sigismember() call in legacy_queue().

> Or in short I think everything turns out ok because the first signal is
> delivered, and the second just happens to get dropped as a duplicate by
> __send_signal.

Right, it is a SIGSEGV in both cases. So it is a legacy signal, and
that'll get marked in that sigset->sig array. Which is per task... ok.

> That is fragile and confusing to depend on so we should just fix the
> code to not send the wrong signal.

Yap.

> I hope that clears things up.

Very much so, thanks for taking the time!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette