From nobody Thu Sep 12 09:03:05 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4X4BL44zMyz5Vt2c; Thu, 12 Sep 2024 09:03:08 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4X4BL444cnz4hqP; Thu, 12 Sep 2024 09:03:08 +0000 (UTC) (envelope-from jhb@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1726131788; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=a0d7Iyo2Vp5WVJIqhWAkIV4PSnp5RVHaCrmXn6+Exs0=; b=cfEisya6fBu7ivO3kZGgaUc1b+1FRi6Bq+ZmkFwfUmDCXEDFEYNmdNXApRpVYf51wMdJ02 gIcZAUWJ9ZsHcdg3fj/R9hyb8NQ3deHqpCUGTCqjpwm7i0XSqI2UnEmc6nyvYEMonXrG0f j+PRZXvRSdRXxYS/PC2Lp2TlAc0Xq/l/eeNqi0vnqFno8UIENc1z/Q4aGoBo/N7NlNVNzg bZlhgVIY5HWG3aVQfHU71nDR9azZU87YB67kQRDLwf7xBtRQNRvukTfE8ltibDZxqUqera lYNMqusMOzBg3b+mWT7r8H4PKBnPY8X/dJ3KsfA4OvpTuoRInHU30iBgh4NmLw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1726131788; a=rsa-sha256; cv=none; b=fbohFeoF7qV/D70O4mXKF1ZOGn4QmD7Zt+dc+ATmlc46FYK0Xfwhwhr5EVCxqU3vqeUaLo 39po2+nhT6DHci310QSZ+4zkFAW+FDLQX48NULLu7lG4ozYHr1+SeLEwonvqdqVNZSddo9 1aM0wuAQSghuIF4pIOdlebvYH8jbmqUGwiYQJqexea0SBd2XAc/J/iQK5d3YowevNv9gqW S7F8KklfM+bNSUAUNMe4OvEHg6lt6wnPG3YpRtlKBkU8Mcntl1RMSm80B7ST3pnf6JXLiM n7cU8Fft7M5uFcVG7AUS7HDN2acaIJXQZd5941VczXWNtWWc7XgyMWf5b95UKw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1726131788; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=a0d7Iyo2Vp5WVJIqhWAkIV4PSnp5RVHaCrmXn6+Exs0=; b=jEFrqM96zOnYq0uALhAIM4IEIYQ23eG8JTTgHMPnFE3lkz0VRYijsJObTClaHxJITKUGF2 osrZUA+iBhh+fHm0pokgWpCXnEYfCb3GCUmqsK4ZW4MDXoIqy8lsZFVy0OzsfTEc4xxmuo OS4J2MMqRbnqdguPejHySxJuomURHQOjlqa47Yq8EZhwo+ZpfMlSJlJhDXK2X+gfvvXyMe r3N5s6Ly5I//ofW/0F8GAnwOZ5UnnQnEnEMGaMDoGbJAdosr+T+LcObC1pgmtZ3QIELqh4 Y9cU6wA2ZEt5DgamvE34S19eP8AmzKykVe52Z3kOSSre2GpmKca2zqOikewK9g== Received: from [10.252.223.137] (unknown [131.111.5.201]) (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 did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 4X4BL367F1zMT3; Thu, 12 Sep 2024 09:03:07 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <2b1955e2-fbf1-41cb-b256-a9a257b16a83@FreeBSD.org> Date: Thu, 12 Sep 2024 10:03:05 +0100 List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: git: f08247fd888e - main - Assert that mbufs are writable if we write to them Content-Language: en-US To: Drew Gallatin , Kristof Provost , Mark Johnston Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202409111118.48BBIQ2h065089@gitrepo.freebsd.org> From: John Baldwin In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit I think part of the motivation for marking M_EXTPG as read-only is that you can't "write" to m_data via mtod() or the like. That said, M_EXTPG aren't really read-only. It depends on the backing store. M_EXTPG were first merged into FreeBSD prior to KTLS to support sendfile, and in that case, they should be M_RDONLY because they alias pages from the file's VM object. However, M_EXTPG mbufs allocated via functions like m_uiotombuf_nomap should not be M_RDONLY. I think this originated in the original import of KTLS which doesn't push setting M_RDONLY out to the callers of mb_alloc_extpgs, and a few other places that hardcode M_RDONLY with M_EXTPG (_mb_unmapped_to_ext should preserve M_RDONLY from the original mbuf instead of forcing M_RDONLY). I can take a stab at a patch but won't have time to really test it until after Euro. On 9/11/24 16:56, Drew Gallatin wrote: > M_EXTPGS mbufs are marked read-only because they refer to external data. The original crypto code, (before kTLS was converted to OCF), used to just build an iovec using PHYS_TO_DMAP() on the page array. I think this case was missed during the conversion to OCF. > > I'm not sure what the best thing to do is, as they should be read only, except this one specific case.... I'd be tempted to just nerf the KASSERT for EXTPGS. > > On Wed, Sep 11, 2024, at 11:02 AM, Kristof Provost wrote: >> On 11 Sep 2024, at 16:45, Mark Johnston wrote: >>> On Wed, Sep 11, 2024 at 11:18:26AM +0000, Kristof Provost wrote: >>>> The branch main has been updated by kp: >>>> >>>> URL: https://cgit.FreeBSD.org/src/commit/?id=f08247fd888e6f7db0ecf2aaa39377144ac40b4c >>>> >>>> commit f08247fd888e6f7db0ecf2aaa39377144ac40b4c >>>> Author: Kristof Provost >>>> AuthorDate: 2024-09-10 20:15:31 +0000 >>>> Commit: Kristof Provost >>>> CommitDate: 2024-09-11 11:17:48 +0000 >>>> >>>> Assert that mbufs are writable if we write to them >>>> >>>> m_copyback() modifies the mbuf, so it must be a writable mbuf. >>> >>> This change still triggers a panic for me when running KTLS tests. I >>> note that EXTPG mbufs always have M_RDONLY set, but I'm not quite sure >>> why. I suspect such mbufs need special handling with respect to the new >>> assertion. >>> >>> syzbot also triggered this panic: >>> https://syzkaller.appspot.com/bug?extid=58c918369f9dc323409d >>> >> Yeah, I saw that one before I went out for a bike ride. >> >> Clearly something is wrong. Either ktls is using read-only buffers or the M_WRITABLE() macro isn’t quite smart enough to spot this specific case. >> >> I’m not familiar enough with ktls to easily tell which. >> >> I’ll back this assertion change out for now, so we’re not panicing test machines while we figure this out. >> >> Best regards, >> Kristof >> > -- John Baldwin