Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <14f85d24-a9de-9706-32f0-30be4999c71c@oracle.com>
Date:   Tue, 31 May 2022 15:07:04 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 2/2] procfs: Add 'path' to /proc/<pid>/fdinfo/
Content-Language: en-US
To:     Kalesh Singh <kaleshsingh@google.com>
Cc:     ilkos@google.com, tjmercier@google.com, surenb@google.com,
        kernel-team@android.com, Jonathan Corbet <corbet@lwn.net>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        Christoph Anton Mitterer <mail@christoph.anton.mitterer.name>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Colin Cross <ccross@google.com>,
        Mike Rapoport <rppt@kernel.org>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-media@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org
References: <20220531212521.1231133-1-kaleshsingh@google.com>
 <20220531212521.1231133-3-kaleshsingh@google.com>
From:   Stephen Brennan <stephen.s.brennan@oracle.com>
In-Reply-To: <20220531212521.1231133-3-kaleshsingh@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RDUyMllCMWpQeDN5aDE5aTcya2o1N1hJUEs1V3NnTHRaQXBvaU93RWhYbkNE?=
 =?utf-8?B?Z1EreEpSc2J3Q1lSNzVwUktWU0RDOGhXVXgxdzVvMDg2WFBCdk9TZFV5ZkdO?=
 =?utf-8?B?N3ZSZnEvK3E5QVZpbFRFclZ4UUZWdmVSTHkrazdka1QzdGc1TzRGclFJcHdG?=
 =?utf-8?B?QXB0MkZvS3p3LzhrWlZyNWtobzVmdkRKKzNhSGNhVFc5T2RvdHlWMzV5WWhx?=
 =?utf-8?B?YnVPR2htRkFUUWg3UFM1dFliOG5yZ0VCNERXSElJK05kcHhxRi9ueVZyV2JC?=
 =?utf-8?B?eFpvWGVjcWF0SmJOdyswM3MvT3ViaXdYa3g3elFqVHJ0Q3gzc2Z0OW1OVHFs?=
 =?utf-8?B?Rmh6Y1Z5UU5BSVBCNEEzdFNCNDhxN3hMYVlLUFNxTENIZXBZYTBMeW5XUHpM?=
 =?utf-8?B?L1U3Wk1meTltRGY0dmg5S2M0bkdNaWNrSmh1SnFmTzY1WDJlUi90bElORjVQ?=
 =?utf-8?B?d2R3M2dpdnJYaTBZZlQrdkJNdG1NUzRMSUk5NFNYV2RnVy9FQ3VVT2NXSCtJ?=
 =?utf-8?B?ejJ3OTM4RFZUNTE4ZjhwUWlDeWVvaGVaR2I0Y2E5TzNyZWZ2cWJFYTloZ3FE?=
 =?utf-8?B?ZzFjTGVpS3pteURHVnpVRW9lL2dIR25kK3hVZUlNekRxUW5nOUlPRzR5SlZ6?=
 =?utf-8?B?SEY2d28wa2hPTU1UVGZVYVR6RVd6Zk5YUGdwUkF4a1dwTjhkdUlmZTBCRlRu?=
 =?utf-8?B?M2s1L3o3c2owcU1jam42bUpEU1BSOUlLbHlNNjF4Wko2dW5iMUlUK2hKdFFy?=
 =?utf-8?B?S3Q0NXhZVkNXcnZZK1ozTHl0R0MvM3VvSWhNZm1YZEhVaHg3ZDl4YWJYV1kr?=
 =?utf-8?B?V2V5MDFiaDQ1NmFTWHl5M1ZrZTRPeEVhMzRCUUV0ckMrbUZqTFJBeVJvVFNR?=
 =?utf-8?B?ZmhHb2dybGhYbmZLWU5vQVVESHRWY01jYVAzMzlZRHBTSVM3R25nQ3RRUVRH?=
 =?utf-8?B?dTQ5VzdXVSs3eFVKMlljSU5QTlhpaVRnRitmS0FidCtyMDdpamxLMFVaM0V2?=
 =?utf-8?B?STR4bk1DQjFiOE9ybFpxb05sM2lOR2JaUmFPK2hBN0FCb0RTcU9waXY5QXlE?=
 =?utf-8?B?UFd4MW9LdlliMWdPMFl4bzhjZ1dBSkpzeVB5VGpOSjd6RE8xUW4xWXVhVWdH?=
 =?utf-8?B?NktXYi9rOVN6QWxNclplakkvaWN6UTd3bXFpMjNZOENNVTdaZkhjclZUMVEw?=
 =?utf-8?B?bmtnTDZCOUgrNHNRMTZZL0pKVEV0WUVnTmdkYlpONExscTE0WVg5N2NySUJl?=
 =?utf-8?B?TU1maFlmOXhKTTBuT3N3MkgrYTFLVURpUDZOVXNBNDZwTVk5eFlRVGQxcUQz?=
 =?utf-8?B?MENSSTJKQzVpN0VkL3JCaTEwSURGcktNZWxJeVVlclhjT3RKck5tUTdJM3pS?=
 =?utf-8?B?R1lKVHdhU2Z3a1Z4dFNEZ3VwZ0xmNXZpSHlydDB3c2ZDTlZVRFA3VUFnRXdz?=
 =?utf-8?B?dkp5bHpLWUxqcUVuRWd6VjNEL1JacFpxR2t0K3VBNlVvOTN6MXk0cXNLTnB0?=
 =?utf-8?B?UGRQVHpmMXFiQUYxU2h0Q3BPZmxVM1BJTVExVmkrYjFqQ0swTFkwYmNxcy9J?=
 =?utf-8?B?eUNkS09zK0xRMDJtbHZSa2Qxc3pQdEczRXlMUkVFME1aZDl6ZURnbzAvSnZK?=
 =?utf-8?B?V0hQZU0zY0Q5dWEwNzlPd1RCcjBiV1lOUUdCaks4TzVHYmJxczBrWVd6SVBP?=
 =?utf-8?B?MWVQTmNMMjNFUFVTZlk0V2hOaVptUElpY21XNGtnQy9JSWY1bERnOGdtSGZ1?=
 =?utf-8?B?UDBOU0ZZeG5wd2x4dU9jS1o1Mm9XQnJKVGdHb1FEb3FCM09taEVpS2tBbVUy?=
 =?utf-8?B?UTZiS09QdUJHSTUrcnRwWWJheGpqTGppY2ZnNFdtR2VVdGN6VzQ2OWh3dlI5?=
 =?utf-8?B?cExFTmFrTStDOTlHZFhkbXh4Ri90Qzg4N29BWW5pc3NkK1MrZkNWR1E1QUhv?=
 =?utf-8?B?L2kweHZnMTNwMk9RYUdIT3JyZWtyUjVmUWRROHRVSkpadWxOU1phMkVmWTkv?=
 =?utf-8?B?MEwwTmVXRjBSQ1ZSZnZySWQwN1MwR3VYYU1PMUxNaEdVdkI1dWtKaHJvWTRX?=
 =?utf-8?B?c094a3l6L2ZZVFlBMElpblBMVDlsN0ZQWW5oYWVFWEY5R1dkNkJqUC94elN5?=
 =?utf-8?B?RmtuRFZWVXlBb0RrUmd4MElnVzBwdzRYZ0d0RmxBN1ZrRkc1SVRXSW1xa25a?=
 =?utf-8?B?UkpMczlYelZaZzBhMC85WlcyUEovSVRFMXpRN2NxejMwMGJTSllXZjArZTBx?=
 =?utf-8?B?cHZpMWU3RCtHYXVNd3lOWXhpd1BycDVhZzVyNncrL0Q1cDJpdXlkS25rV1FT?=
 =?utf-8?B?eUNhUmJvREVOU1dPYzJIOG5MdG8zSWxEamEvOHM2MGZUa0lGN1dxa2hmcG9H?=
 =?utf-8?Q?bOVGGRTKWfFUMOis=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 880be326-65ba-41bb-5f06-08da4351e75d
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 May 2022 22:07:08.1757
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: i5ZAQcWuY2B/ggyBq9GjqFzqOtGSQ2aaw2KuoA7Aq8OBkkeN7CPbsXbTCXZW/I2KCWN5ab9uKF9A6ja2/gFTXO8rQpxwISzQQUKsBCtaVDU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB3408
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.874
 definitions=2022-05-31_08:2022-05-30,2022-05-31 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 bulkscore=0
 mlxlogscore=999 malwarescore=0 mlxscore=0 spamscore=0 phishscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2204290000 definitions=main-2205310096
X-Proofpoint-ORIG-GUID: vtGIMmriu2B-m-nMXLrecZZNJvN2t0kz
X-Proofpoint-GUID: vtGIMmriu2B-m-nMXLrecZZNJvN2t0kz
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable 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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5/31/22 14:25, Kalesh Singh wrote:
> In order to identify the type of memory a process has pinned through
> its open fds, add the file path to fdinfo output. This allows
> identifying memory types based on common prefixes. e.g. "/memfd...",
> "/dmabuf...", "/dev/ashmem...".
> 
> Access to /proc/<pid>/fdinfo is governed by PTRACE_MODE_READ_FSCREDS
> the same as /proc/<pid>/maps which also exposes the file path of
> mappings; so the security permissions for accessing path is consistent
> with that of /proc/<pid>/maps.

Hi Kalesh,

I think I see the value in the size field, but I'm curious about path,
which is available via readlink /proc/<pid>/fd/<n>, since those are
symlinks to the file themselves.

File paths can contain fun characters like newlines or colons, which
could make parsing out filenames in this text file... fun. How would your
userspace parsing logic handle "/home/stephen/filename\nsize:\t4096"? The
readlink(2) API makes that easy already.

Is the goal avoiding races (e.g. file descriptor 3 is closed and reopened
to a different path between reading fdinfo and stating the fd)?

Stephen

> Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
> ---
> 
> Changes from rfc:
>   - Split adding 'size' and 'path' into a separate patches, per Christian
>   - Fix indentation (use tabs) in documentaion, per Randy
> 
>  Documentation/filesystems/proc.rst | 14 ++++++++++++--
>  fs/proc/fd.c                       |  4 ++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 779c05528e87..591f12d30d97 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -1886,14 +1886,16 @@ if precise results are needed.
>  3.8	/proc/<pid>/fdinfo/<fd> - Information about opened file
>  ---------------------------------------------------------------
>  This file provides information associated with an opened file. The regular
> -files have at least five fields -- 'pos', 'flags', 'mnt_id', 'ino', and 'size'.
> +files have at least six fields -- 'pos', 'flags', 'mnt_id', 'ino', 'size',
> +and 'path'.
>  
>  The 'pos' represents the current offset of the opened file in decimal
>  form [see lseek(2) for details], 'flags' denotes the octal O_xxx mask the
>  file has been created with [see open(2) for details] and 'mnt_id' represents
>  mount ID of the file system containing the opened file [see 3.5
>  /proc/<pid>/mountinfo for details]. 'ino' represents the inode number of
> -the file, and 'size' represents the size of the file in bytes.
> +the file, 'size' represents the size of the file in bytes, and 'path'
> +represents the file path.
>  
>  A typical output is::
>  
> @@ -1902,6 +1904,7 @@ A typical output is::
>  	mnt_id:	19
>  	ino:	63107
>  	size:	0
> +	path:	/dev/null
>  
>  All locks associated with a file descriptor are shown in its fdinfo too::
>  
> @@ -1920,6 +1923,7 @@ Eventfd files
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:[eventfd]
>  	eventfd-count:	5a
>  
>  where 'eventfd-count' is hex value of a counter.
> @@ -1934,6 +1938,7 @@ Signalfd files
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:[signalfd]
>  	sigmask:	0000000000000200
>  
>  where 'sigmask' is hex value of the signal mask associated
> @@ -1949,6 +1954,7 @@ Epoll files
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:[eventpoll]
>  	tfd:        5 events:       1d data: ffffffffffffffff pos:0 ino:61af sdev:7
>  
>  where 'tfd' is a target file descriptor number in decimal form,
> @@ -1968,6 +1974,7 @@ For inotify files the format is the following::
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:inotify
>  	inotify wd:3 ino:9e7e sdev:800013 mask:800afce ignored_mask:0 fhandle-bytes:8 fhandle-type:1 f_handle:7e9e0000640d1b6d
>  
>  where 'wd' is a watch descriptor in decimal form, i.e. a target file
> @@ -1992,6 +1999,7 @@ For fanotify files the format is::
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:[fanotify]
>  	fanotify flags:10 event-flags:0
>  	fanotify mnt_id:12 mflags:40 mask:38 ignored_mask:40000003
>  	fanotify ino:4f969 sdev:800013 mflags:0 mask:3b ignored_mask:40000000 fhandle-bytes:8 fhandle-type:1 f_handle:69f90400c275b5b4
> @@ -2018,6 +2026,7 @@ Timerfd files
>  	mnt_id:	9
>  	ino:	63107
>  	size:   0
> +	path:	anon_inode:[timerfd]
>  	clockid: 0
>  	ticks: 0
>  	settime flags: 01
> @@ -2042,6 +2051,7 @@ DMA Buffer files
>  	mnt_id:	9
>  	ino:	63107
>  	size:   32768
> +	path:	/dmabuf:
>  	count:  2
>  	exp_name:  system-heap
>  
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index 464bc3f55759..8889a8ba09d4 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -60,6 +60,10 @@ static int seq_show(struct seq_file *m, void *v)
>  	seq_printf(m, "ino:\t%lu\n", file_inode(file)->i_ino);
>  	seq_printf(m, "size:\t%lli\n", (long long)file_inode(file)->i_size);
>  
> +	seq_puts(m, "path:\t");
> +	seq_file_path(m, file, "\n");
> +	seq_putc(m, '\n');
> +
>  	/* show_fd_locks() never deferences files so a stale value is safe */
>  	show_fd_locks(m, file, files);
>  	if (seq_has_overflowed(m))