From owner-svn-src-head@freebsd.org Wed Dec 2 00:48:52 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 626B746C09A; Wed, 2 Dec 2020 00:48:52 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cm0hg4kVpz3hLR; Wed, 2 Dec 2020 00:48:51 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x330.google.com with SMTP id a3so1229975wmb.5; Tue, 01 Dec 2020 16:48:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=eLrb9aoyb/Zjpc66m/L2QNsk+TuT7/eCpMQIiePFFj8=; b=XlJZycFoYp/EpQW230tP2pn92A0g6Ep4AW+YtMiHxRQaf9d12t7qPkb17FUL/QaKpK igg0ZJecjhwzgYmB6dQ1uFaa1XlQZgJpett0ndRcwe4iKxcid5nMz/UmLVlBUeEzP4lz ehyZXePfZPB/+3aleDRAM4RHA6QFCVe3ueV8OoNEas/mb9hrY3SrAAN8jLbu6TICB5y7 g3nkAurknS7R809Erwf2yMhm6jTL3vKXhv6fZ21hj1JifWudNxcCCGFz0+a3pOAeLe9N ZhHQuskQbyuU8+hBzCH0aO9OXrSUJVHETN02cfT+apRINFGEG/U/4H5MRbXvcWiv9dC9 XphA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=eLrb9aoyb/Zjpc66m/L2QNsk+TuT7/eCpMQIiePFFj8=; b=qDPChP74U6ceh32MnlJkapBNzZpiP44DmvRgCFVijXbI+QzPQY6Z9WuYkKL8CXHXph IBicDomn9gDOjpbMBT5YlUM/7D41kADIdAEFVYV20Q6ql3UbGMVK9yR4WP/lxtdUMpao QBz7Kl2OCCa21wZQK/QE5LyBciVul8CO9H7RXa0+i2AMwAiBSqGgmnGcoquh5CeOuDut e4DH4FQdvt4o/cNACQBbk50/U2VYzXy3LuuvwLrIQB+PxIezDXXwNrlG1gUncEEG3/7W mv1D0ekVwJvvegK7OTtV7qg7KiKSbmB2hkZevC4XHUaP8ozsCN/RpANYgiABqjBeVJqA 1Dyg== X-Gm-Message-State: AOAM530b4SG93xCqonidTyEybhijRSig8FgfblBNhKKcXXo5csbj9LFp aSkFLmOAqT8EJDLrftKcGTIWPKvGcuXmS7Wt4lI+Rkxl/Xw= X-Google-Smtp-Source: ABdhPJz5lOvmDTCrszLKY3w1SXqPtWnwqgF1jQ75ONxilTBjiVQgkW14AUOR1DvzS3BHosHX44J6w7Fyz2D+Ts0HDIc= X-Received: by 2002:a7b:c012:: with SMTP id c18mr274101wmb.10.1606870129819; Tue, 01 Dec 2020 16:48:49 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a5d:4d47:0:0:0:0:0 with HTTP; Tue, 1 Dec 2020 16:48:49 -0800 (PST) In-Reply-To: <202012020048.0B20mFQo098578@repo.freebsd.org> References: <202012020048.0B20mFQo098578@repo.freebsd.org> From: Mateusz Guzik Date: Wed, 2 Dec 2020 01:48:49 +0100 Message-ID: Subject: Re: svn commit: r368271 - head/sys/kern To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4Cm0hg4kVpz3hLR X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20161025 header.b=XlJZycFo; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of mjguzik@gmail.com designates 2a00:1450:4864:20::330 as permitted sender) smtp.mailfrom=mjguzik@gmail.com X-Spamd-Result: default: False [-2.00 / 15.00]; ARC_NA(0.00)[]; RBL_DBL_DONT_QUERY_IPS(0.00)[2a00:1450:4864:20::330:from]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_SPF_ALLOW(-0.20)[+ip6:2a00:1450:4000::/36:c]; FREEMAIL_FROM(0.00)[gmail.com]; MIME_GOOD(-0.10)[text/plain]; TO_DN_NONE(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; TO_MATCH_ENVRCPT_ALL(0.00)[]; SPAMHAUS_ZRD(0.00)[2a00:1450:4864:20::330:from:127.0.2.255]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_SPAM_SHORT(1.00)[0.999]; DKIM_TRACE(0.00)[gmail.com:+]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; RCVD_IN_DNSWL_NONE(0.00)[2a00:1450:4864:20::330:from]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[svn-src-all,svn-src-head]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Dec 2020 00:48:52 -0000 On 12/2/20, Mateusz Guzik wrote: > Author: mjg > Date: Wed Dec 2 00:48:15 2020 > New Revision: 368271 > URL: https://svnweb.freebsd.org/changeset/base/368271 > > Log: > select: make sure there are no wakeup attempts after selfdfree returns > > Prior to the patch returning selfdfree could still be racing against > doselwakeup > which set sf_si = NULL and now locks stp to wake up the other thread. > > A sufficiently unlucky pair can end up going all the way down to freeing > select-related structures before the lock/wakeup/unlock finishes. > > This started manifesting itself as crashes since select data started > getting > freed in r367714. > Reported by: hps, mike tancsa > Modified: > head/sys/kern/sys_generic.c > > Modified: head/sys/kern/sys_generic.c > ============================================================================== > --- head/sys/kern/sys_generic.c Wed Dec 2 00:45:35 2020 (r368270) > +++ head/sys/kern/sys_generic.c Wed Dec 2 00:48:15 2020 (r368271) > @@ -1820,14 +1820,17 @@ doselwakeup(struct selinfo *sip, int pri) > */ > TAILQ_REMOVE(&sip->si_tdlist, sfp, sf_threads); > stp = sfp->sf_td; > - /* > - * Paired with selfdfree. > - */ > - atomic_store_rel_ptr((uintptr_t *)&sfp->sf_si, (uintptr_t)NULL); > mtx_lock(&stp->st_mtx); > stp->st_flags |= SELTD_PENDING; > cv_broadcastpri(&stp->st_wait, pri); > mtx_unlock(&stp->st_mtx); > + /* > + * Paired with selfdfree. > + * > + * Storing this only after the wakeup provides an invariant that > + * stp is not used after selfdfree returns. > + */ > + atomic_store_rel_ptr((uintptr_t *)&sfp->sf_si, (uintptr_t)NULL); > } > mtx_unlock(sip->si_mtx); > } > @@ -1837,14 +1840,18 @@ seltdinit(struct thread *td) > { > struct seltd *stp; > > - if ((stp = td->td_sel) != NULL) > - goto out; > - td->td_sel = stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); > + stp = td->td_sel; > + if (stp != NULL) { > + MPASS(stp->st_flags == 0); > + MPASS(STAILQ_EMPTY(&stp->st_selq)); > + return; > + } > + stp = malloc(sizeof(*stp), M_SELECT, M_WAITOK|M_ZERO); > mtx_init(&stp->st_mtx, "sellck", NULL, MTX_DEF); > cv_init(&stp->st_wait, "select"); > -out: > stp->st_flags = 0; > STAILQ_INIT(&stp->st_selq); > + td->td_sel = stp; > } > > static int > @@ -1887,6 +1894,8 @@ seltdfini(struct thread *td) > stp = td->td_sel; > if (stp == NULL) > return; > + MPASS(stp->st_flags == 0); > + MPASS(STAILQ_EMPTY(&stp->st_selq)); > if (stp->st_free1) > free(stp->st_free1, M_SELFD); > if (stp->st_free2) > -- Mateusz Guzik