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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D152AC433E9 for ; Fri, 29 Jan 2021 07:16:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8716B64E03 for ; Fri, 29 Jan 2021 07:16:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232226AbhA2HQB (ORCPT ); Fri, 29 Jan 2021 02:16:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32820 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230388AbhA2HP5 (ORCPT ); Fri, 29 Jan 2021 02:15:57 -0500 Received: from mail-wm1-x332.google.com (mail-wm1-x332.google.com [IPv6:2a00:1450:4864:20::332]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9EE4EC061574 for ; Thu, 28 Jan 2021 23:15:16 -0800 (PST) Received: by mail-wm1-x332.google.com with SMTP id u14so6027270wml.4 for ; Thu, 28 Jan 2021 23:15:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iLdacsm3XogIsaiExukBUmqPzsJY8TDJ1FbcWEsqSm4=; b=axhlIo/D+wi8wYEr5mSU2TeU2BHoww+YZeD6E6GdrBwgl9gpG9ta/zxED6w811vA1e XUkx3ytL3FDQNK19YaMIEfgRJ9aJTyOqVg4+lHSJSxmM/EXiU0PQeTncr79rkwfJI3As rS0N5aN3Puio1orZOigpQI40abIOPc4XPhctpY411MUxXy/iO7Ha1wmCcwBa9I+sr8kh z2fSowH8P0SCc6TKI1/wZMphf47/x3IAmMNlXetDpHKsjLgiu9uOK97lKzrZ6fPGCh86 4Hg+FaVgBIqtMK8TmAQN6CW2XmGYez8FXgs1HYOoQcmOJwObMajdXLB5rgANhUDDeeXq 0SdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iLdacsm3XogIsaiExukBUmqPzsJY8TDJ1FbcWEsqSm4=; b=ZjipfexJaLb7niebA1C9TjNqJPz0qGC21HWsIJJl1gBTa4mr5P4X0BOm/8TkHzX12E 14Col+lpTsT7f9AitGDgbpJT2NHIew3TdgUWtxEVqZCyZ6CXSFtUP58ICyLkyHZk3n/p wTPJMUMzQ+MeJHcVNa6P7sBOylahXX+USPcdv3npbrY+nA9fQbGlq3+cumkawPzEi0yN AiXti4clamDCN783uv36VjCw/G2db1gmNBtCw07Pegby8TgWRPHmj0KAGU+XyR4QBllM eoie3I6Nlr7Sj+qZYX8ji7xg7EVgcsUIeS+4KEIB9kL3FBN5/Tjm4MQ5+zDQAAKlqYmB ErrA== X-Gm-Message-State: AOAM533SyHMy3oiweCKJXxefu7ZgwyByJXFWCQXbQcWUiO3Zb5YOQZOb y8TEWx5LOMHnpsq3yBKq5hdujQvBUs2i4SwEuZmeQQ== X-Google-Smtp-Source: ABdhPJxpkxr3lWUY1Rn0hsphGiX4XcyWnJxlD176nyKrLed2lzKaKrzUHbmSNYf+8M3ImhOcw+T4C3o2/WEbt+7Lfus= X-Received: by 2002:a1c:7906:: with SMTP id l6mr2443143wme.22.1611904515127; Thu, 28 Jan 2021 23:15:15 -0800 (PST) MIME-Version: 1.0 References: <20210120202337.1481402-1-surenb@google.com> <6cd84701-fb65-7aa0-38db-b69fe5748754@gmail.com> In-Reply-To: <6cd84701-fb65-7aa0-38db-b69fe5748754@gmail.com> From: Suren Baghdasaryan Date: Thu, 28 Jan 2021 23:15:04 -0800 Message-ID: Subject: Re: [PATCH 1/1] process_madvise.2: Add process_madvise man page To: "Michael Kerrisk (man-pages)" Cc: linux-man , Andrew Morton , Jann Horn , Kees Cook , Jeffrey Vander Stoep , Minchan Kim , Michal Hocko , Shakeel Butt , David Rientjes , =?UTF-8?Q?Edgar_Arriaga_Garc=C3=ADa?= , Tim Murray , Linux-MM , SElinux list , linux-security-module , Linux API , lkml , Android Kernel Team Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 28, 2021 at 12:31 PM Michael Kerrisk (man-pages) wrote: > > Hello Suren, > > On 1/28/21 7:40 PM, Suren Baghdasaryan wrote: > > On Thu, Jan 28, 2021 at 4:24 AM Michael Kerrisk (man-pages) > > wrote: > >> > >> Hello Suren, > >> > >> Thank you for writing this page! Some comments below. > > > > Thanks for the review! > > Couple questions below and I'll respin the new version once they are clarified. > > Okay. See below. > > >> On Wed, 20 Jan 2021 at 21:36, Suren Baghdasaryan wrote: > >>> > > [...] > > Thanks for all the acks. That let's me know that you saw what I said. > > >>> RETURN VALUE > >>> On success, process_madvise() returns the number of bytes advised. This > >>> return value may be less than the total number of requested bytes, if an > >>> error occurred. The caller should check return value to determine whether > >>> a partial advice occurred. > >> > >> So there are three return values possible, > > > > Ok, I think I see your point. How about this instead: > > Well, I'm glad you saw it, because I forgot to finish it. But yes, > you understood what I forgot to say. > > > RETURN VALUE > > On success, process_madvise() returns the number of bytes advised. This > > return value may be less than the total number of requested bytes, if an > > error occurred after some iovec elements were already processed. The caller > > should check the return value to determine whether a partial > > advice occurred. > > > > On error, -1 is returned and errno is set appropriately. > > We recently standardized some wording here: > s/appropriately/to indicate the error/. > > > >>> +.PP > >>> +The pointer > >>> +.I iovec > >>> +points to an array of iovec structures, defined in > >> > >> "iovec" should be formatted as > >> > >> .I iovec > > > > I think it is formatted that way above. What am I missing? > > But also in "an array of iovec structures"... > > > BTW, where should I be using .I vs .IR? I was looking for an answer > > but could not find it. > > .B / .I == bold/italic this line > .BR / .IR == alternate bold/italic with normal (Roman) font. > > So: > .I iovec > .I iovec , # so that comma is not italic > .BR process_madvise () > etc. > > [...] > > >>> +.I iovec > >>> +if one of its elements points to an invalid memory > >>> +region in the remote process. No further elements will be > >>> +processed beyond that point. > >>> +.PP > >>> +Permission to provide a hint to external process is governed by a > >>> +ptrace access mode > >>> +.B PTRACE_MODE_READ_REALCREDS > >>> +check; see > >>> +.BR ptrace (2) > >>> +and > >>> +.B CAP_SYS_ADMIN > >>> +capability that caller should have in order to affect performance > >>> +of an external process. > >> > >> The preceding sentence is garbled. Missing words? > > > > Maybe I worded it incorrectly. What I need to say here is that the > > caller should have both PTRACE_MODE_READ_REALCREDS credentials and > > CAP_SYS_ADMIN capability. The first part I shamelessly copy/pasted > > from https://man7.org/linux/man-pages/man2/process_vm_readv.2.html and > > tried adding the second one to it, obviously unsuccessfully. Any > > advice on how to fix that? > > I think you already got pretty close. How about: > > [[ > Permission to provide a hint to another process is governed by a > ptrace access mode > .B PTRACE_MODE_READ_REALCREDS > check (see > BR ptrace (2)); > in addition, the caller must have the > .B CAP_SYS_ADMIN > capability. In V2 I explanded a bit this part to explain why CAP_SYS_ADMIN is needed. There were questions about that during my patch review which adds this requirement (https://lore.kernel.org/patchwork/patch/1363605), so I thought a short explanation would be useful. > ]] > > [...] > > >>> +.TP > >>> +.B ESRCH > >>> +No process with ID > >>> +.I pidfd > >>> +exists. > >> > >> Should this maybe be: > >> [[ > >> The target process does not exist (i.e., it has terminated and > >> been waited on). > >> ]] > >> > >> See pidfd_send_signal(2). > > > > I "borrowed" mine from > > https://man7.org/linux/man-pages/man2/process_vm_readv.2.html but > > either one sounds good to me. Maybe for pidfd_send_signal the wording > > about termination is more important. Anyway, it's up to you. Just let > > me know which one to use. > > I think the pidfd_send_signal(2) wording fits better. > > [...] > > Thanks, > > Michael > > -- > Michael Kerrisk > Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ > Linux/UNIX System Programming Training: http://man7.org/training/