Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <7e862b1c-7818-6759-caf1-962598d2c8b3@fb.com>
Date:   Wed, 2 Mar 2022 12:55:06 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.1
Subject: Re: [PATCH bpf-next v1 1/9] bpf: Add mkdir, rmdir, unlink syscalls
 for prog_bpf_syscall
Content-Language: en-US
To:     Hao Luo <haoluo@google.com>, Alexei Starovoitov <ast@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>
Cc:     Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
        KP Singh <kpsingh@kernel.org>,
        Shakeel Butt <shakeelb@google.com>,
        Joe Burton <jevburton.kernel@gmail.com>,
        Tejun Heo <tj@kernel.org>, joshdon@google.com, sdf@google.com,
        bpf@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220225234339.2386398-1-haoluo@google.com>
 <20220225234339.2386398-2-haoluo@google.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20220225234339.2386398-2-haoluo@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dDdoT2FwdnoxMk5YcGppOEVSUE5FYUZlNi9HN1h5UU5OSE1zQ25qWVlML2hZ?=
 =?utf-8?B?dTFER1l6ZmVTazlkZm51aFc1dlk0OWxTaFYwcXJXcFVxRm1obFhVWmNIK21T?=
 =?utf-8?B?QlV5SzYybjBVMWZUMlpXcFVJWEVZdGl2YnpXclZhTmpvZDAvRE5CYnNJZU5X?=
 =?utf-8?B?L3dYWTR3QTJHbGVwQUFPRjFCVjMrbFJyYmVjdVBYQk0rSW1SUEdxeDFjYmFp?=
 =?utf-8?B?RmZxYUYrcjlQYmRlcEpqK0syb3E3WDRLYThXZkFMcVMxemhQK1lZN1lsZ25a?=
 =?utf-8?B?dU55VW1wbFl2TjU0UHpqVVNicUxVZ3B2d1kwOFRZcDNyTFZMMk9VemhoZGFq?=
 =?utf-8?B?RWNmYjM3Wm9yQmI5cHRPVlZSd1A2TU9kcExwT0NjWkhEUytsTXRsMzlMQkd2?=
 =?utf-8?B?cmF2OHZKTDhHMTlVdVY0ZDgvUnhnQ1N0MDNldmRpcFZVRzZMUG9RcTk5dzhB?=
 =?utf-8?B?U3RNY0VWSWxqSVQ2MFJacEJ5bGtVdjNhWGFoVDlZZlhjelVWbWNEQUltb3BJ?=
 =?utf-8?B?UEJZWWk0RklyTzFPL1NOOFdTRDUwL09kNEI2ZGhmZk1rY0hndVNncHNXN1JK?=
 =?utf-8?B?c1F4aTFaWlErUFc4d1d3QnlLQXozckM2OUxpVitPOVM3TFgyVFlGZVRBWVZM?=
 =?utf-8?B?a2hQQXVEcGN2VWVaZjlqd1pjSG01SFBUdTFHaTFURWhwNnpocElkb1N0V0dS?=
 =?utf-8?B?UzV0NFdqVXYzSmdiK3BQWG13T0w2U2RVSlNUSVdQMlc0UU1jZTJlQzJ5dUl4?=
 =?utf-8?B?TTExdDMvK1R5a0pOMGZ3N2tJbk0vMnA0S0F2bWQ0V0IxQ1Vra1hZUFYzd0Vo?=
 =?utf-8?B?TVRXMjRDUVNmRWFnSE80UlpjM0NReG1qbEJQbFlmTERLVnNwZTh4b0ZZcnlZ?=
 =?utf-8?B?QTNVTWxPVVdHaU1LemxlZSs4dUZ6MW1XK1JwdzYycUhEdFB1amdwbGlraHpv?=
 =?utf-8?B?VkgvT0QzbEVBeU5LQU9VOHp2M3dHNWNGVXVNemRDYys5TkhNYnZPeE5SMnZr?=
 =?utf-8?B?WnlQczlXYWdPMTM5K1pRTVZXRHlnTm5xWS90ZGdSa01IYW9oZVIxdzVIVVE4?=
 =?utf-8?B?K0c1RGRBYjZCTDRTOEtyQU9NRW01eXViQmkzQUlYTU9heXpXTnY0cTJKYVBP?=
 =?utf-8?B?M2lJVGpsQ3EwaEl0T3l2WHB5RFFjUytGeUM4L3N5cE9mNWUwdnZwcDlXbXZC?=
 =?utf-8?B?RmdXc0w2M3NrdVdUUExkK0s3aVZKZnV6SC9ZMzRzUFJ5eCtrSEhkeWZKenlI?=
 =?utf-8?B?dTVKQlJBOEVySSs0S1poRWI1RXp6aEZqeTFON1B4ejZzYWQ0Wkhldlp5WkMr?=
 =?utf-8?B?bDcwZnFGemYzeWVFZnhjOXNpRk5LNS9NVjk1cGVQRThVaFlhT0Zma3huWlY4?=
 =?utf-8?B?VU0vejl1Y1pkckZhNmlBMFMzQ0JYZzJjK2ZsSWlpUUpQdnk1UDRZalN4UWRK?=
 =?utf-8?B?V1NjOFMxN1QrQ3JLcW1ybmk1TlJiMFhWNzdMQXl1bDQ0NlM5UzZiK1VIWjdv?=
 =?utf-8?B?RjNodVF0UUF3ZXpZSVQ4YS81cTgwdW91U1FQREhVaVMrK0dMckZsS3k4V21h?=
 =?utf-8?B?TmFKdTFlNWtkV0gxZDdLUk5DcXM5NFRIU090NnU2OU9EaUNRMjl1dXRid0Q2?=
 =?utf-8?B?blhrU3FjR3ROaFJCUmtoTVlpYmVIUmI5VTlTZHYxTWN2dGVKMTdTSFdpT1Fo?=
 =?utf-8?B?N2UwUWNKaTJjOXR4V0hWS2FFZmJ2bDIwM2tBaHVlTnNaMU1yaWdDcEJaemQ2?=
 =?utf-8?B?MThJZ2lLQ2owdDJlOU1lY2hoRE1VV0VhQkx3bGF6NlM2WXVUNDFSdjEyMDly?=
 =?utf-8?B?ZWFwNGlOZlVKUTBaN1Vrak5TQXcvY3kxNDRmcU56Yk5UbFVScFV5K0hFZkt1?=
 =?utf-8?B?Qmgxb0FkTkc4LzZuVE5iY1kxRXhRNjZiMjFKdVlOaHhZTTJveEtjS09BemF4?=
 =?utf-8?B?bHBOeXkwL3oxb2FJVjFEMXJnVHFwMDhjaVVPT25tb1VkcDU4Zk9oQTMzc1B3?=
 =?utf-8?B?cW10cHc4L1lwTEFzU2dNNHB4QnhQU3g4MVBMYmlvWFRFalRDcXVUeG9zZXJs?=
 =?utf-8?B?WWZ2M2I4amtsTGQrRjgzS2tlaGkzUExzalI1b0IzL3BVc1VhVjVwWHF3dmFE?=
 =?utf-8?Q?nu8g0tiD4Oayr3uvo4pcx4ViA?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b7b20daa-64fd-407d-43fd-08d9fc8ef1c1
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Mar 2022 20:55:12.2728
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fwtOPDforTzbU9Xbvtk/eIeeCc0RzDONptyxp2ezm8pwb5Q0kny7p+sqvu+ws4nO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR15MB2452
X-Proofpoint-GUID: OGdfvtAvvdAZscg1IPd5VHef1_FIiW9h
X-Proofpoint-ORIG-GUID: OGdfvtAvvdAZscg1IPd5VHef1_FIiW9h
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.64.514
 definitions=2022-03-02_12,2022-02-26_01,2022-02-23_01
X-Proofpoint-Spam-Details: rule=fb_outbound_notspam policy=fb_outbound score=0 lowpriorityscore=0
 adultscore=0 clxscore=1011 spamscore=0 mlxscore=0 suspectscore=0
 mlxlogscore=999 impostorscore=0 phishscore=0 bulkscore=0 malwarescore=0
 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2201110000 definitions=main-2203020087
X-FB-Internal: deliver
X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	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 2/25/22 3:43 PM, Hao Luo wrote:
> This patch allows bpf_syscall prog to perform some basic filesystem
> operations: create, remove directories and unlink files. Three bpf
> helpers are added for this purpose. When combined with the following
> patches that allow pinning and getting bpf objects from bpf prog,
> this feature can be used to create directory hierarchy in bpffs that
> help manage bpf objects purely using bpf progs.
> 
> The added helpers subject to the same permission checks as their syscall
> version. For example, one can not write to a read-only file system;
> The identity of the current process is checked to see whether it has
> sufficient permission to perform the operations.
> 
> Only directories and files in bpffs can be created or removed by these
> helpers. But it won't be too hard to allow these helpers to operate
> on files in other filesystems, if we want.
> 
> Signed-off-by: Hao Luo <haoluo@google.com>
> ---
>   include/linux/bpf.h            |   1 +
>   include/uapi/linux/bpf.h       |  26 +++++
>   kernel/bpf/inode.c             |   9 +-
>   kernel/bpf/syscall.c           | 177 +++++++++++++++++++++++++++++++++
>   tools/include/uapi/linux/bpf.h |  26 +++++
>   5 files changed, 236 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index f19abc59b6cd..fce5e26179f5 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1584,6 +1584,7 @@ int bpf_link_new_fd(struct bpf_link *link);
>   struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd);
>   struct bpf_link *bpf_link_get_from_fd(u32 ufd);
>   
> +bool bpf_path_is_bpf_dir(const struct path *path);
>   int bpf_obj_pin_user(u32 ufd, const char __user *pathname);
>   int bpf_obj_get_user(const char __user *pathname, int flags);
>   
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index afe3d0d7f5f2..a5dbc794403d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5086,6 +5086,29 @@ union bpf_attr {
>    *	Return
>    *		0 on success, or a negative error in case of failure. On error
>    *		*dst* buffer is zeroed out.
> + *
> + * long bpf_mkdir(const char *pathname, int pathname_sz, u32 mode)

Can we make pathname_sz to be u32 instead of int? pathname_sz should 
never be negative any way.

Also, I think it is a good idea to add 'u64 flags' parameter for all
three helpers, so we have room in the future to tune for new use cases.

> + *	Description
> + *		Attempts to create a directory name *pathname*. The argument
> + *		*pathname_sz* specifies the length of the string *pathname*.
> + *		The argument *mode* specifies the mode for the new directory. It
> + *		is modified by the process's umask. It has the same semantic as
> + *		the syscall mkdir(2).
> + *	Return
> + *		0 on success, or a negative error in case of failure.
> + *
> + * long bpf_rmdir(const char *pathname, int pathname_sz)
> + *	Description
> + *		Deletes a directory, which must be empty.
> + *	Return
> + *		0 on sucess, or a negative error in case of failure.
> + *
> + * long bpf_unlink(const char *pathname, int pathname_sz)
> + *	Description
> + *		Deletes a name and possibly the file it refers to. It has the
> + *		same semantic as the syscall unlink(2).
> + *	Return
> + *		0 on success, or a negative error in case of failure.
>    */
>   #define __BPF_FUNC_MAPPER(FN)		\
>   	FN(unspec),			\
> @@ -5280,6 +5303,9 @@ union bpf_attr {
>   	FN(xdp_load_bytes),		\
>   	FN(xdp_store_bytes),		\
>   	FN(copy_from_user_task),	\
> +	FN(mkdir),			\
> +	FN(rmdir),			\
> +	FN(unlink),			\
>   	/* */
>   
>   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
[...]