From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73AE4E7717F for ; Wed, 11 Dec 2024 02:39:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E8AA58D0017; Tue, 10 Dec 2024 21:39:04 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E3A456B00C9; Tue, 10 Dec 2024 21:39:04 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CB3868D0017; Tue, 10 Dec 2024 21:39:04 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id AC05A6B00C7 for ; Tue, 10 Dec 2024 21:39:04 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id B308FAE3B3 for ; Wed, 11 Dec 2024 02:39:03 +0000 (UTC) X-FDA: 82881120756.14.0C6E923 Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) by imf09.hostedemail.com (Postfix) with ESMTP id 5F8C2140004 for ; Wed, 11 Dec 2024 02:38:46 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=MaR3eFCq; spf=pass (imf09.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.210.45 as permitted sender) smtp.mailfrom=jeffxu@chromium.org; dmarc=pass (policy=none) header.from=chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733884718; h=from:from:sender: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:dkim-signature; bh=Vma5UC4Up8IXip1IFD7Lr4sLcxtr+U7Nv0cS4zQrrMg=; b=vhLYfoGuI7rvlRC9fkd6dtPATTQtC3hA5SKQ5NcI+ix+Vjv+4SJhJXQZFLJtIqNeq+9GJY 8peOKm6TQP1VGt/wUPV+y3sadSbhPlGrOVGO1UskfjNZAcFCIa+kXBPwRIodUvMVaTzWwL SFSu7SiHm3B4yc4znl9ns74cc0WrJxI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733884718; a=rsa-sha256; cv=none; b=ldZs7/T+uAXcS/Zbu6XWWA5HvH9+Lmvzxdeuf3AEN3JYY19bQ8a2yi5b0k6QBzj8QKbMZ8 Zydwu4kp3DHzzgzW7W4fOww9cQWO/bapOmEi+zRqRKFtiqgWle4uESP2yEnNlvNp4tMGze hB/gJjsRQMq1YfO0HhtwpY7PkymyB3I= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=MaR3eFCq; spf=pass (imf09.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.210.45 as permitted sender) smtp.mailfrom=jeffxu@chromium.org; dmarc=pass (policy=none) header.from=chromium.org Received: by mail-ot1-f45.google.com with SMTP id 46e09a7af769-71e15abd163so96124a34.0 for ; Tue, 10 Dec 2024 18:39:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1733884740; x=1734489540; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Vma5UC4Up8IXip1IFD7Lr4sLcxtr+U7Nv0cS4zQrrMg=; b=MaR3eFCqcgfMzMF/jV8hmmSACMMJBVUkVDCGsPOe9rkMcyVQ0ZhyVA+ZZxFvPKvKRZ ooA8vGSgwRpu2hLpnujv3TzgWc/9u32uckpr2SWNDUDV8AKGjiXgJE6bBy4KRN0rUjWb aB3MvdL4ZIO02ONkSf3KXd+tciqK6yV7bk7M0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733884740; x=1734489540; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Vma5UC4Up8IXip1IFD7Lr4sLcxtr+U7Nv0cS4zQrrMg=; b=CPh8m0p3Qp8umyY631RPRp4QM+ufGo5mshhohx297SWz2LQOwg91JRrh2NQkYtOzI4 CJMkSY05bLRyUB3leKmv/DuAPL/8pnP1bgCx5TBCZaInSlhUvpVEUQYNmnvhPF1Vtbps /bIzVicqo7EXQTwmcYkI8ZR6qBOPq6gPmPmRT8ROU3FegSiGvwSCeH27HofCWIQ4riA3 0h7oA3CCk+imSRFk7lop+O8D/zb+luptn10vyUs0LHQ5GaibsZ5VvfRitAW33JAwPC8o gF+I+29zlhqq+ioLUlYXHZZCW9TUBip/IINb3PynDYLVM2KTsjfaPQBx6DaASVJf5fSn FIbQ== X-Forwarded-Encrypted: i=1; AJvYcCWh9WgH/RywWdClbSvM2QmGhshOkipgVQim1yyck3/CZpZ5EOSSc/dokjR/yhJa/0MpajuknZ8dWA==@kvack.org X-Gm-Message-State: AOJu0YwzIfZV9A1AA+klAcqh7OkSSRfgJQU1zyCvDkTNVxF735ILH9mU IMHU+Xulmd1f6NiV70ODxvyW6+7ZwH+6r1zSo2vixjh4SkS7PiR2PBaPIFNqFnVpUN3tDdcW0Sc kEt6np+3J/0m+Dt4iGuY4y+0hF3vS9lR+U5lf X-Gm-Gg: ASbGncstrDM3LaVh+aLTLWy+uJu0ToGzJKUv+QoVX4vgwGu34MZmpedfwOpqtRsLQBS +mYnbeGKPa8gQSCqRBSZ7uikWWCzE8rINgqjI6t6/B9UN7yDkeD53mCyZ1YZ3xZj/ X-Google-Smtp-Source: AGHT+IEZFASK6PUZ+4WyunObGGitl8fk62QLOo0Za5h2KjoAHplUqp/QgRz8ZkDfBfQe8U5z7n2k0R9zDrd7o2X6lIk= X-Received: by 2002:a05:6870:b48a:b0:29e:5525:f4fd with SMTP id 586e51a60fabf-2a012f874bdmr271231fac.14.1733884740297; Tue, 10 Dec 2024 18:39:00 -0800 (PST) MIME-Version: 1.0 References: <20241206013934.2782793-1-jeffxu@google.com> <2m3zmlehfigs5r7rptwcoft3j4fipfkgfxmdrdttpf76hwhwae@vclfa5ulcmv2> In-Reply-To: From: Jeff Xu Date: Tue, 10 Dec 2024 18:38:49 -0800 Message-ID: Subject: Re: [PATCH v1] mseal: move can_do_mseal to mseal.c To: Lorenzo Stoakes Cc: "Liam R. Howlett" , akpm@linux-foundation.org, vbabka@suse.cz, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, linux-mm@kvack.org, jorgelo@chromium.org, keescook@chromium.org, pedro.falcato@gmail.com, rdunlap@infradead.org, Jann Horn , David Hildenbrand Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 5F8C2140004 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: 7wzodabwet7d85g6cwrfyobc7pjqz6uu X-HE-Tag: 1733884726-77966 X-HE-Meta: U2FsdGVkX1+mJfgDHQcQLC5MURHne2kcnhd/YZZSFpOihchIAvyDQVJZ6nxGR7c5utpO6AZ/pbp+AKwf3/9PAXk7fB8BpjXFYN+f7vtU4GlYROvaLZ1/STCdo+FcGh4cuF9iTP3mHHiXVU3RV37DdzH41/2l0r2ohDRspHmDaziUjc/Y5svVbbWNMHkemaTlP5wrg0nDcdxWgMcjaFUT4CO/0GNnLHA0+SDgSXx5v2rNk8uirpcTG+KvgOIRTEOV6Wk46GFZ233mMxqZv+EDlRjBc5R6OtsPBWnDP+RYVJg+JWfl59P5OkvR/f55ezl5hNvNU/ekbLZf2d19cbi+3pSh0J0ONnql6S6Gwt1Wnes//IUzYMB7uzfDdyInWpQKfcXuQHvANXzqaOs1Gm3NPdy0pkL2XaU/ZkFQzQq77lLIciAh1MmnYQP307NmOompjnFri/ctxMR/Lp9Wy4cB5c3u6JQFMx5o/M5jYYGlwdJC7OGEl5q4LqveaQs90tl5+ui6eF6cZqiNHyp2iTnnsrrLQWoADJ2/DrbEIAM5ub3X8bHOEGTjs0rIYZaVHFVvg+Xc7CkdKsym12yU0zh4cxZ7++cQ8KeEvlfrN/qjCJC4BzGzzCACvAg2X55mQjYuJRA8fgUKJRfi0fdSGtsknOF4ViS0n3VoIicj8+xsHf/kYPlpNW+UqZG4jA+lOFjspB536kLeOyDzeoUdEq+CjIMkkZKlZmPVU6E9dT138CHHnvx3OXWczqaa3V69La/aUX/s9B4ow+Ye/01XRPNjCm9Efe0PqRe/VTvt6+tmEugeBZRwD5GiQdZLaLJi+fDpoPZl3Dtt5a15/Fe5ZNVfNAfYZGOuO7IusoRT72KE+3PlNWliKd/dtSCl7qN1J5HpB9oBNbMQIPqS+yvASJV+rQLfEbMp0QRDpTc9dPfDI9KKCZzNOxjfJH6aMTUIgHTdYhQfxUFN94Knm+PQARt BM/8IFbO oSLxtMbG7zYhc4+O9NI6l22LY8wQKgVcUHDe46ZTo/BRjlRw+2GHuD3NfHzrXpMZAmXJ5+GiXTzPr1b5KKQOeLdGQtX476IXgWLNVUWqRdXmSUwa8zIWwkAn5njm6W4aQZBSemVXziblMoR6o7usG1NBq7Nvl07FJ3d2d0tTOo873alWZRlpFyAEWNLaIXYags3Nnqc7Wm/S5erGP3KisC6+OLzj/4/pzEZDzM6BmiORLEVUy/Z5pCEPbblfuKAxXtbv7z9E3bz+cv65TvKqA9V8II04tWpnlkjDRcVv3hfr0KOao2ki0HSSqL2WWmoaxfyvFa2U5EW9Y/4Fn9P/OQEKOd+dZDqhwMlRuic2wFt0yIW0= X-Bogosity: Ham, tests=bogofilter, spamicity=0.446819, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Lorenzo, Regarding your proposal of moving mseal.c to vma.c for unit testing. On Fri, Dec 6, 2024 at 9:04=E2=80=AFAM Lorenzo Stoakes wrote: > > > > > > An aside - I actually think we need to move the bulk of this code to > > > mm/vma.c - it makes absolutely no sense to keep the internals in this= file, > > > and that way we can userland test mseal functionality. > > > > > Is there a past discussion to read ? That will help me understand your > > strategy of unit testing mm code. > > Moving everything to vma.c, will lose log history, e.g. blame no > > longer helps, did we consider alternatives ? > > Re; git blame - I'm not sure what alternative you think exists, and I've > moved brk(), mmap(), etc. with a history spanning >30 years, so I'm not > sure what blame history you're concerned about given how recent mseal is = :) > > There is always code that gets moved or changed. You can't stay attached = to > your name appearing on a git blame line. > > Re: discussion, there's dozens of discussions and patch sets totalling ~3= k > lines of code... just search lore for vma testing, or search through my > commits in mm/vma.c and you can see. > > I can put together links if you really need, but I think say [0] is a goo= d > motivating example of how I was able to actually write unit tests for VMA > merge functionality which previously could not exist. > > In any case you can use the git blame -C option to 'see through' things l= ike > code moves. > > The whole point of this is to be able to _unit_ test functionality under > circumstances that might be otherwise improbable/incredibly difficult to > obtain if run as part of a kernel and self testing. > > Importantly it allows us to conduct fuzzing testing in future, something > key and fundamental to security testing. > > I would say for somebody who has clearly stated his huge commitment to > testing and how critically vital it is especially in the security realm, > this is entirely something that is beneficial to the kernel and to mseal > stability and security. > > If you want to see it 'in action', you can run the tests in > tools/testing/vma via: > > $ make && ./vma > I want to express my support for unit testing and agree that more testing would benefit mm. However, I'm unsure about the reasoning behind moving code to vma.c in bulk. Could you please clarify this for me? In my understanding, unit tests can be conducted regardless of the code's location once dependencies are addressed with stubs. Have you considered adding mseal.c to the unittest makefile at the same level as vma.c? Since mseal.c doesn't introduce new dependencies, i.e. it operates directly on the vm_area_struct, so I would start with that. I guess, for UT, you might need to change some functions' signatures, e.g. remove static, if you want to test an internal function (e.g mseal_fixup) , from your unit-test, but this is the same even after moving them to vma.c. There will be additional work of clean up including header (".h"), still I believe this is the same work even after moving the code into vma.c. You might still need to move the prototype of some functions into vma.h or vma_internal.h (e.g. definition of MADV_FREE). But I think this work is also orthogonal to where the mseal business logic is located. I understand the logic behind the current vma.c (on the linux_main branch) and the unit test for the VMA merge functionality. However, if your plan is to move all VMA-related code into vma.c, that means more stubs are needed (depending on the boundary of the proposed unit testing), and I don't understand how moving the code can help reduce the amount of work or stubs (if that is the motivation). To avoid spending too much of your time, if there are previous discussions on this topic, please share links or a brief summary, so I can study them first. Thanks! Best Regards, -Jeff > [0]https://lore.kernel.org/linux-mm/1c7a0b43cfad2c511a6b1b52f3507696478ff= 51a.1725040657.git.lorenzo.stoakes@oracle.com/ > > Thanks, Lorenzo