From patchwork Thu Jun 1 18:58:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 102144 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp553185vqr; Thu, 1 Jun 2023 12:08:01 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7mO9fW7iWO/6GvH1Qhc01Vt4dRBObMVDyrkC9Vq8b4ysU6LdVrKeSyGrVK8QKdIqRuWi8D X-Received: by 2002:a17:902:e84c:b0:1af:e2f2:1cce with SMTP id t12-20020a170902e84c00b001afe2f21ccemr299760plg.38.1685646480957; Thu, 01 Jun 2023 12:08:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685646480; cv=none; d=google.com; s=arc-20160816; b=mj6Uyl4mLhHbU61UbPMiGow+8YwmHivLc8ztaw+ElyAQowihCYvY8sywzGPXCPo8wp 9dcAh9B/qORKqp7RQf1IRA+eyxvp4fijyiOJbMcgF5KWbiIlaErb4ShphiWaK2VPGuCq HCkaZZjmlnO1+2G9sdJjBbKOqn727iijtwPds2A3S/T57vg7U+RsjspNC2LONNuPfWBh h5TV1FSRtgqBuJT3OW2rARo36prH+R6EbiRAgAIMjP9ZdYZxOGc3NvabaobzyPbeScep v//lgi7ykoIoorVL/7nNA1k13z5idXZzoFUl8D9d6vcu5qKWNDMZZnhFI6qnsto58UkM fDtA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:references :in-reply-to:subject:cc:to:dkim-signature:dkim-signature:from; bh=tSw2aFlx/hQ0bMemqwh60SU+YcRXUfbeW4+63KudMSQ=; b=oVHHUfaKBuCVJcPdbKLKkvppWUW6c70ezfxD8WmP+KcBOLtaOTDpOK/i3xY6fZXj/u hUTWL33oZKo762URRMAEn0RxpOjLvFB6CfoqzDueEJ7j3fsq5xp9W5XMR8MZdv+bcwoY +yPteoKI5i4Ed7T1wnKLmR4MexLJNT1+VyaOBxJ/JAD+mYaHrK0VOLqaTcVFYVKOpN6C tzTQw9EUhQ79K52LEccL6/4U1QIG6AR4l34cZ69TwzBNb9b3GkdUTUvIOcufNBnls/mv vSVkQt2vVkDYqFZIkTA3Ob5v1Ze2KSm6TQsaz6Xg9CfiPcLpG6UKGbJBQjRoY4t/loWx /cfw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=i53gbr4Z; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ja22-20020a170902efd600b001b03ba09cd1si2993919plb.128.2023.06.01.12.07.42; Thu, 01 Jun 2023 12:08:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=i53gbr4Z; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231166AbjFAS6w (ORCPT + 99 others); Thu, 1 Jun 2023 14:58:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229490AbjFAS6v (ORCPT ); Thu, 1 Jun 2023 14:58:51 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9377D184 for ; Thu, 1 Jun 2023 11:58:49 -0700 (PDT) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1685645927; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tSw2aFlx/hQ0bMemqwh60SU+YcRXUfbeW4+63KudMSQ=; b=i53gbr4Z5ZP/4rrzfhFh6/K3tlOu+nZRQgkB0F2GZXOxYQKMo6Gmt6azZZhS0MLRaBSKGk 00zX6rwuxzHHBKI6pIZo0thXzM8uN2aAMYTddOuLPdz8SbW/DrKs/6UBBgNDbbYPrzg87y CmWtGlhfNt/WxehpvGMQ+9hjQBUvn1wNsMDBrkCDk38Nz4/d8Qm/eWdgwmheHAPnthZCEs 2HHLZWAO99nwYlDfX5aouf2W301+nmFpTjI9tmhnKAf+3qNeCXe4OcRP5In9B0Zm2dXWpJ IdNH7rE+GnwyPLyT48UCtJ9WWe1JSEUgfg+UkwUFZEJPBf5MW9P9qn8wM9rHXg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1685645927; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=tSw2aFlx/hQ0bMemqwh60SU+YcRXUfbeW4+63KudMSQ=; b=/DUzf4GoPkeZu+c3kGJ5jEuPjdd4ybznLsZwPo1HZQccnpzr3Jil4WDpuJswNS8bpxjAU4 88iNwCvmjXE1mbAQ== To: Frederic Weisbecker Cc: LKML , Anna-Maria Behnsen , Peter Zijlstra , syzbot+5c54bd3eb218bb595aa9@syzkaller.appspotmail.com, Dmitry Vyukov , Sebastian Siewior , Michael Kerrisk Subject: [patch v2 02/20] posix-timers: Ensure timer ID search-loop limit is valid In-Reply-To: References: <20230425181827.219128101@linutronix.de> <20230425183312.932345089@linutronix.de> <87zg6i2xn3.ffs@tglx> <87v8h62vwp.ffs@tglx> <877cth1xyd.ffs@tglx> Date: Thu, 01 Jun 2023 20:58:47 +0200 Message-ID: <87bkhzdn6g.ffs@tglx> MIME-Version: 1.0 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764176368182784315?= X-GMAIL-MSGID: =?utf-8?q?1767528444823559803?= posix_timer_add() tries to allocate a posix timer ID by starting from the cached ID which was stored by the last successful allocation. This is done in a loop searching the ID space for a free slot one by one. The loop has to terminate when the search wrapped around to the starting point. But that's racy vs. establishing the starting point. That is read out lockless, which leads to the following problem: CPU0 CPU1 posix_timer_add() start = sig->posix_timer_id; lock(hash_lock); ... posix_timer_add() if (++sig->posix_timer_id < 0) start = sig->posix_timer_id; sig->posix_timer_id = 0; So CPU1 can observe a negative start value, i.e. -1, and the loop break never happens because the condition can never be true: if (sig->posix_timer_id == start) break; While this is unlikely to ever turn into an endless loop as the ID space is huge (INT_MAX), the racy read of the start value caught the attention of KCSAN and Dmitry unearthed that incorrectness. Rewrite it so that all id operations are under the hash lock. Reported-by: syzbot+5c54bd3eb218bb595aa9@syzkaller.appspotmail.com Reported-by: Dmitry Vyukov Signed-off-by: Thomas Gleixner Reviewed-by: Frederic Weisbecker --- V2: Make the loop less hideous. --- include/linux/sched/signal.h | 2 +- kernel/time/posix-timers.c | 31 ++++++++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -135,7 +135,7 @@ struct signal_struct { #ifdef CONFIG_POSIX_TIMERS /* POSIX.1b Interval Timers */ - int posix_timer_id; + unsigned int next_posix_timer_id; struct list_head posix_timers; /* ITIMER_REAL timer for the process */ --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -140,25 +140,30 @@ static struct k_itimer *posix_timer_by_i static int posix_timer_add(struct k_itimer *timer) { struct signal_struct *sig = current->signal; - int first_free_id = sig->posix_timer_id; struct hlist_head *head; - int ret = -ENOENT; + unsigned int cnt, id; - do { + /* + * FIXME: Replace this by a per signal struct xarray once there is + * a plan to handle the resulting CRIU regression gracefully. + */ + for (cnt = 0; cnt <= INT_MAX; cnt++) { spin_lock(&hash_lock); - head = &posix_timers_hashtable[hash(sig, sig->posix_timer_id)]; - if (!__posix_timers_find(head, sig, sig->posix_timer_id)) { + id = sig->next_posix_timer_id; + + /* Write the next ID back. Clamp it to the positive space */ + sig->next_posix_timer_id = (id + 1) & INT_MAX; + + head = &posix_timers_hashtable[hash(sig, id)]; + if (!__posix_timers_find(head, sig, id)) { hlist_add_head_rcu(&timer->t_hash, head); - ret = sig->posix_timer_id; + spin_unlock(&hash_lock); + return id; } - if (++sig->posix_timer_id < 0) - sig->posix_timer_id = 0; - if ((sig->posix_timer_id == first_free_id) && (ret == -ENOENT)) - /* Loop over all possible ids completed */ - ret = -EAGAIN; spin_unlock(&hash_lock); - } while (ret == -ENOENT); - return ret; + } + /* POSIX return code when no timer ID could be allocated */ + return -EAGAIN; } static inline void unlock_timer(struct k_itimer *timr, unsigned long flags)