linux-kernel - Re: crisv32 runtime failure in -next due to 'page-flags: define behavior SL*B-related flags on compound pages'

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 23 Sep 2015 08:02:44 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	"Kirill A. Shutemov" <kirill@...temov.name>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	akpm@...ux-foundation.org
Cc:	Hans-Peter Nilsson <hans-peter.nilsson@...s.com>, starvik@...s.com,
	jespern@...s.com, hughd@...gle.com,
	kirill.shutemov@...ux.intel.com, linux-next@...r.kernel.org,
	linux-kernel@...r.kernel.org, minchan@...nel.org,
	linux-cris-kernel@...s.com
Subject: Re: crisv32 runtime failure in -next due to 'page-flags: define
 behavior SL*B-related flags on compound pages'

On 09/23/2015 03:53 AM, Kirill A. Shutemov wrote:
> On Tue, Sep 22, 2015 at 08:40:14AM -0700, Paul E. McKenney wrote:
>> On Tue, Sep 22, 2015 at 06:31:04PM +0300, Kirill A. Shutemov wrote:
>>> On Tue, Sep 22, 2015 at 08:18:35AM -0700, Paul E. McKenney wrote:
>>>> On Tue, Sep 22, 2015 at 03:57:06PM +0200, Hans-Peter Nilsson wrote:
>>>>> I guess you hit the right spot, but I'd think people would be
>>>>> more comfortable with aligning to sizeof (void *).
>>>>
>>>> I would indeed prefer sizeof(void *).
>>>
>>> Do you prefer to have the attribute set for whole structure or for ->next?
>>> I think attribute on ->next is more appropriate from documentation POV.
>
> I retract this claim: we have requirement about pointee alignment, not
> pointer alignment.
>
>>>From edbab9e89f5e4ad42e63d93ab05519e6a5f4d552 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> Date: Wed, 23 Sep 2015 13:39:28 +0300
> Subject: [PATCH] rcu: force alignment on struct callback_head/rcu_head
>
> This patch makes struct callback_head aligned to size of pointer. On
> most architectures it happens naturally due ABI requirements, but some
> architectures (like CRIS) have weird ABI and we need to ask it
> explicitly.
>
> The alignment is required to guarantee that bits 0 and 1 of @next will
> be clear under normal conditions -- as long as we use call_rcu(),
> call_rcu_bh(), call_rcu_sched(), or call_srcu() to queue callback.
>
> This guarantee is important for few reasons:
>   - future call_rcu_lazy() will make use of lower bits in the pointer;
>   - the structure shares storage spacer in struct page with @compound_head,
>     which encode PageTail() in bit 0. The guarantee is needed to avoid
>     false-positive PageTail().
>
> False postive PageTail() caused crash on crisv32[1]. It happend due
> misaligned task_struct->rcu, which was byte-aligned.
>
> [1] http://lkml.kernel.org/r/55FAEA67.9000102@roeck-us.net
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Guenter Roeck <linux@...ck-us.net>

Tested-by: Guenter Roeck <linux@...ck-us.net>

Hope the patch won't get lost since it was attached to an e-mail.
Can it be added to the branch introducing the problem ?

Thanks,
Guenter

> ---
>   include/linux/types.h | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/types.h b/include/linux/types.h
> index c314989d9158..70d8500bddf1 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -205,11 +205,25 @@ struct ustat {
>    * struct callback_head - callback structure for use with RCU and task_work
>    * @next: next update requests in a list
>    * @func: actual update function to call after the grace period.
> + *
> + * The struct is aligned to size of pointer. On most architectures it happens
> + * naturally due ABI requirements, but some architectures (like CRIS) have
> + * weird ABI and we need to ask it explicitly.
> + *
> + * The alignment is required to guarantee that bits 0 and 1 of @next will be
> + * clear under normal conditions -- as long as we use call_rcu(),
> + * call_rcu_bh(), call_rcu_sched(), or call_srcu() to queue callback.
> + *
> + * This guarantee is important for few reasons:
> + *  - future call_rcu_lazy() will make use of lower bits in the pointer;
> + *  - the structure shares storage spacer in struct page with @compound_head,
> + *    which encode PageTail() in bit 0. The guarantee is needed to avoid
> + *    false-positive PageTail().
>    */
>   struct callback_head {
>   	struct callback_head *next;
>   	void (*func)(struct callback_head *head);
> -};
> +} __attribute__((aligned(sizeof(void *))));
>   #define rcu_head callback_head
>
>   typedef void (*rcu_callback_t)(struct rcu_head *head);
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/