[PATCH 5/5] erofs-utils: mount: stop checking `/sys/block/nbdX/pid`

[Gao Xiang]

Hi Yifan,

On 2025/12/23 18:04, Yifan Zhao wrote:
> The `current erofsmount_nbd()` implementation verifies that the value in
> `/sys/block/nbdX/pid`` matches the PID of the process executing
> `erofsmount_nbd_loopfn()`, using this as an indicator that the NBD
> device is ready. This check is incorrect, as the PID recorded in the
> aforementioned sysfs file may belong to a kernel thread rather than a
> userspace process (see [1]).

Do you have a way to reproduce that?

> 
> Moreover, since this verification only occurs after the child process
> has successfully issued and returned from the NBD connect request,
> removing it introduces no risk of NBD device hijacking by malicious
> actors. This patch removes the erroneous check.

It's not used to avoid device hijacking by malicious actors but
detecting if the NBD device is reused by another daemon.

Thanks,
Gao Xiang

> 
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/block/nbd.c#L1501
> 
> Signed-off-by: Yifan Zhao <zhaoyifan28 at huawei.com>
> ---
>   lib/backends/nbd.c | 16 +++++-----------
>   lib/liberofs_nbd.h |  2 +-
>   mount/main.c       |  5 ++---
>   3 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/lib/backends/nbd.c b/lib/backends/nbd.c
> index 46e75cd..2d941a9 100644
> --- a/lib/backends/nbd.c
> +++ b/lib/backends/nbd.c
> @@ -52,7 +52,8 @@ struct nbd_reply {
>   	};
>   } __packed;
>   
> -long erofs_nbd_in_service(int nbdnum)
> +/* Return: 0 while nbd is in service, <0 otherwise */
> +int erofs_nbd_in_service(int nbdnum)
>   {
>   	int fd, err;
>   	char s[32];
> @@ -72,17 +73,10 @@ long erofs_nbd_in_service(int nbdnum)
>   		return -ENOTCONN;
>   
>   	(void)snprintf(s, sizeof(s), "/sys/block/nbd%d/pid", nbdnum);
> -	fd = open(s, O_RDONLY);
> -	if (fd < 0)
> +	if (access(s, F_OK) < 0)
>   		return -errno;
> -	err = read(fd, s, sizeof(s));
> -	if (err < 0) {
> -		err = -errno;
> -		close(fd);
> -		return err;
> -	}
> -	close(fd);
> -	return strtol(s, NULL, 10);
> +
> +	return 0;
>   }
>   
>   int erofs_nbd_devscan(void)
> diff --git a/lib/liberofs_nbd.h b/lib/liberofs_nbd.h
> index 78c8af5..b719d80 100644
> --- a/lib/liberofs_nbd.h
> +++ b/lib/liberofs_nbd.h
> @@ -34,7 +34,7 @@ struct erofs_nbd_request {
>   /* 30-day timeout for NBD recovery */
>   #define EROFS_NBD_DEAD_CONN_TIMEOUT	(3600 * 24 * 30)
>   
> -long erofs_nbd_in_service(int nbdnum);
> +int erofs_nbd_in_service(int nbdnum);
>   int erofs_nbd_devscan(void);
>   int erofs_nbd_connect(int nbdfd, int blkbits, u64 blocks);
>   char *erofs_nbd_get_identifier(int nbdnum);
> diff --git a/mount/main.c b/mount/main.c
> index d2d4815..f6cba33 100644
> --- a/mount/main.c
> +++ b/mount/main.c
> @@ -1270,6 +1270,8 @@ static int erofsmount_nbd(struct erofs_nbd_source *source,
>   
>   	while (1) {
>   		err = erofs_nbd_in_service(msg.nbdnum);
> +		if (!err)
> +			break;
>   		if (err == -ENOENT || err == -ENOTCONN) {
>   			err = waitpid(child, &child_status, WNOHANG);
>   			if (err < 0)
> @@ -1280,9 +1282,6 @@ static int erofsmount_nbd(struct erofs_nbd_source *source,
>   			usleep(50000);
>   			continue;
>   		}
> -		if (err >= 0)
> -			err = (err != child ? -EBUSY : 0);
> -		break;
>   	}
>   
>   	if (!err) {