Skip to content

Commit 5ed72a7

Browse files
committed
mountpoint_cmp: flatten mountpoint= paramater before sorting
Datasets with nested mountpoints must be mounted in order from shallowest to deepest, otherwise the deeper ones will be obscured by the shallow ones. mountpoint_cmp() has only traditionally been a character-based sort, with no awareness of "legal" patterns in pathnames, in particular, runs of '/' characters. These are (for better or worse) legal paths, but confuse a character-based comparison, causing mounts to be attempted in an incorrect order, with mixed results. This commit adds a path "flattening" function, and applies it to the string returned from the mountpoint= paramater before using it as a path. This corrects the sort order and allows mounting to proceed correctly. Reported-by: James McKay <portablejim@jamesmckay.id.au> Sponsored-by: TrueNAS Signed-off-by: Rob Norris <rob.norris@truenas.com>
1 parent 757c7db commit 5ed72a7

File tree

4 files changed

+96
-0
lines changed

4 files changed

+96
-0
lines changed

include/libzutil.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ _LIBZUTIL_H char *zfs_strip_partition(const char *);
126126
_LIBZUTIL_H const char *zfs_strip_path(const char *);
127127

128128
_LIBZUTIL_H int zfs_strcmp_pathname(const char *, const char *, int);
129+
_LIBZUTIL_H int zfs_flatten_path(char *, size_t, char **);
129130

130131
_LIBZUTIL_H boolean_t zfs_dev_is_dm(const char *);
131132
_LIBZUTIL_H boolean_t zfs_dev_is_whole_disk(const char *);

lib/libzfs/libzfs.abi

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,7 @@
379379
<elf-symbol name='zfs_device_get_physical' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
380380
<elf-symbol name='zfs_dirnamelen' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
381381
<elf-symbol name='zfs_expand_proplist' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
382+
<elf-symbol name='zfs_flatten_path' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
382383
<elf-symbol name='zfs_foreach_mountpoint' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
383384
<elf-symbol name='zfs_get_all_props' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
384385
<elf-symbol name='zfs_get_clones_nvl' type='func-type' binding='global-binding' visibility='default-visibility' is-defined='yes'/>
@@ -5957,6 +5958,12 @@
59575958
<parameter type-id='80f4b756'/>
59585959
<return type-id='48b5725f'/>
59595960
</function-decl>
5961+
<function-decl name='zfs_flatten_path' mangled-name='zfs_flatten_path' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zfs_flatten_path'>
5962+
<parameter type-id='26a90f95'/>
5963+
<parameter type-id='b59d7dce'/>
5964+
<parameter type-id='9b23c9ad'/>
5965+
<return type-id='95e97e5e'/>
5966+
</function-decl>
59605967
<function-decl name='mkdirp' mangled-name='mkdirp' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='mkdirp'>
59615968
<parameter type-id='80f4b756'/>
59625969
<parameter type-id='d50d396c'/>
@@ -6554,6 +6561,11 @@
65546561
<pointer-type-def type-id='35acf840' size-in-bits='64' id='17f3480d'/>
65556562
<pointer-type-def type-id='688c495b' size-in-bits='64' id='cec6f2e4'/>
65566563
<pointer-type-def type-id='d11b7617' size-in-bits='64' id='23432aaa'/>
6564+
<function-decl name='dump_nvlist' visibility='default' binding='global' size-in-bits='64'>
6565+
<parameter type-id='5ce45b60'/>
6566+
<parameter type-id='95e97e5e'/>
6567+
<return type-id='48b5725f'/>
6568+
</function-decl>
65576569
<function-decl name='zpool_get_handle' mangled-name='zpool_get_handle' visibility='default' binding='global' size-in-bits='64' elf-symbol-id='zpool_get_handle'>
65586570
<parameter type-id='4c81de99'/>
65596571
<return type-id='b0382bb3'/>

lib/libzfs/libzfs_mount.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
* Copyright 2017 RackTop Systems.
2929
* Copyright (c) 2018 Datto Inc.
3030
* Copyright 2018 OmniOS Community Edition (OmniOSce) Association.
31+
* Copyright (c) 2026, TrueNAS.
3132
*/
3233

3334
/*
@@ -269,6 +270,7 @@ zfs_is_mountable(zfs_handle_t *zhp, char *buf, size_t buflen,
269270
if (source)
270271
*source = sourcetype;
271272

273+
VERIFY0(zfs_flatten_path(buf, buflen, NULL));
272274
return (B_TRUE);
273275
}
274276

@@ -998,11 +1000,13 @@ mountpoint_cmp(const void *arga, const void *argb)
9981000
if (gota) {
9991001
verify(zfs_prop_get(za, ZFS_PROP_MOUNTPOINT, mounta,
10001002
sizeof (mounta), NULL, NULL, 0, B_FALSE) == 0);
1003+
VERIFY0(zfs_flatten_path(mounta, sizeof (mounta), NULL));
10011004
}
10021005
gotb = (zfs_get_type(zb) == ZFS_TYPE_FILESYSTEM);
10031006
if (gotb) {
10041007
verify(zfs_prop_get(zb, ZFS_PROP_MOUNTPOINT, mountb,
10051008
sizeof (mountb), NULL, NULL, 0, B_FALSE) == 0);
1009+
VERIFY0(zfs_flatten_path(mountb, sizeof (mountb), NULL));
10061010
}
10071011

10081012
if (gota && gotb) {
@@ -1061,10 +1065,13 @@ non_descendant_idx(zfs_handle_t **handles, size_t num_handles, int idx)
10611065

10621066
verify(zfs_prop_get(handles[idx], ZFS_PROP_MOUNTPOINT, parent,
10631067
sizeof (parent), NULL, NULL, 0, B_FALSE) == 0);
1068+
VERIFY0(zfs_flatten_path(parent, sizeof (parent), NULL));
10641069

10651070
for (i = idx + 1; i < num_handles; i++) {
10661071
verify(zfs_prop_get(handles[i], ZFS_PROP_MOUNTPOINT, child,
10671072
sizeof (child), NULL, NULL, 0, B_FALSE) == 0);
1073+
VERIFY0(zfs_flatten_path(child, sizeof (child), NULL));
1074+
10681075
if (!libzfs_path_contains(parent, child))
10691076
break;
10701077
}
@@ -1170,6 +1177,7 @@ zfs_mount_task(void *arg)
11701177

11711178
verify(zfs_prop_get(handles[idx], ZFS_PROP_MOUNTPOINT, mountpoint,
11721179
sizeof (mountpoint), NULL, NULL, 0, B_FALSE) == 0);
1180+
VERIFY0(zfs_flatten_path(mountpoint, sizeof (mountpoint), NULL));
11731181

11741182
if (mp->mnt_func(handles[idx], mp->mnt_data) != 0)
11751183
goto out;
@@ -1187,6 +1195,7 @@ zfs_mount_task(void *arg)
11871195
char child[ZFS_MAXPROPLEN];
11881196
verify(zfs_prop_get(handles[i], ZFS_PROP_MOUNTPOINT,
11891197
child, sizeof (child), NULL, NULL, 0, B_FALSE) == 0);
1198+
VERIFY0(zfs_flatten_path(child, sizeof (child), NULL));
11901199

11911200
if (!libzfs_path_contains(mountpoint, child))
11921201
break; /* not a descendant, return */

lib/libzutil/zutil_device_path.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
/*
2424
* Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
25+
* Copyright (c) 2026, Rob Norris <robn@despairlabs.com>
2526
*/
2627

2728
#include <errno.h>
@@ -203,3 +204,76 @@ zfs_strcmp_pathname(const char *name, const char *cmp, int wholedisk)
203204

204205
return (0);
205206
}
207+
208+
/*
209+
* "Flatten" a UNIX-style path by:
210+
* - collapsing multiple '/' chars to just one
211+
* - removing trailing '/' chars (one or more followed by '\0')
212+
*
213+
* args:
214+
* - src: source buffer
215+
* - srclen: size of source buffer (>= strlen(in)+1)
216+
* - dstp: pointer to destination buffer:
217+
* - if NULL, source buffer is modified in place
218+
* - if set and *dstp is set, output written to *dstp
219+
* - if set and *dstp is NULL, *dstp set to malloc(srclen)
220+
*
221+
* returns:
222+
* - 0 on success. caller must free *dstp is it was allocated (above)
223+
* - ENOMEM: alloc failed
224+
*/
225+
int
226+
zfs_flatten_path(char *src, size_t srclen, char **dstp)
227+
{
228+
char *s, *d, *dstbuf = NULL;
229+
230+
s = src;
231+
if (dstp == NULL)
232+
/* Modify in-place */
233+
d = src;
234+
else if (*dstp != NULL)
235+
/* Use provided dest buffer */
236+
d = *dstp;
237+
else {
238+
/* Allocate dest buffer */
239+
dstbuf = malloc(srclen);
240+
if (dstbuf == NULL)
241+
return (ENOMEM);
242+
d = dstbuf;
243+
}
244+
245+
while (*s != '\0') {
246+
/* Copy char unless in-place and no change */
247+
if (s != d)
248+
*d = *s;
249+
250+
/* Found a slash */
251+
if (*s == '/') {
252+
/* Move past additional slashes */
253+
while (*s == '/')
254+
s++;
255+
256+
/*
257+
* Found null after slash, abort. d still points
258+
* at the slash copied above, which will then be
259+
* nulled after the loop.
260+
*/
261+
if (*s == '\0')
262+
break;
263+
} else
264+
/* Non-slash char copied, move source forward. */
265+
s++;
266+
267+
/* Copied char ok, move dest forward for next iteration */
268+
d++;
269+
}
270+
271+
/* End of path, set dest terminator unless in-place and no move. */
272+
if (s != d)
273+
*d = '\0';
274+
275+
if (dstp != NULL && *dstp == NULL)
276+
*dstp = dstbuf;
277+
278+
return (0);
279+
}

0 commit comments

Comments
 (0)