Bug 2232537

Thanks, the info. is useful, I'll see what I can do about it as soon as I can.

(In reply to Roberto Bergantinos from comment #0)
> Description of problem:
> 
> I opened this problem under rhel8, but on field reproduces under rhel7.
> rhel9 also fails

It will be very hard to convince me this should be fixed in rhel7, going to
rhel8 would be the recommendation at this point.

Cloning the bug for rhel9 is normally what I do when both are affected.

> 
> If we have a share exported with blank+dash on the path, i.e "host:/dir/X -
> Y/", automount crash with
> SIGSEGV if we attempt to mount it. I.e in this case we're reproducing it
> with builtin net map "/net -hosts" :
> 
> NFS server sharing the following paths :
> 
> [root@nfs-server ~]# ls -l /test
> total 0
> drwxr-xr-x. 2 root root 6 Aug 16 16:12 A B     ---> OK
> drwxr-xr-x. 2 root root 6 Aug 16 16:18 A _ B   ---> OK
> drwxr-xr-x. 2 root root 6 Aug 16 15:43 A - B   ---> crash
> 
> // If we try to automount "/test/A - B" path automount crashes since we pass
> a null string to strlen :

Right, I'll fix that but the check is not right either so there's going to
be more to it.

> 
> (gdb) where
> #0  __strlen_sse2_pminub () at
> ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S:38
> #1  0x00007f9cb8e629b0 in mount_subtree (ap=ap@entry=0x55906c379910,
> me=me@entry=0x7f9c98002660, name=name@entry=0x7f9cb25c6d80
> "/net/rhel73/test/A - B", loc=loc@entry=0x0, 
>     options=options@entry=0x7f9c98002890 "", ctxt=ctxt@entry=0x7f9cac001060)
> at parse_sun.c:1202
> #2  0x00007f9cb8e639ed in parse_mount (ap=0x55906c379910, name=<optimized
> out>, name_len=<optimized out>, mapent=<optimized out>, context=<optimized
> out>) at parse_sun.c:1545
> #3  0x00007f9cb3dd77ec in do_parse_mount (ctxt=0x7f9cac001000,
> mapent=0x7f9c98002870 "rhel73:/test/A - B", name_len=22, name=0x7f9cb25c6d80
> "/net/rhel73/test/A - B", source=0x55906c379ae0, ap=0x55906c379910)
>     at lookup_hosts.c:166
> #4  lookup_mount (ap=0x55906c379910, name=0x7f9cb25c6d80 "/net/rhel73/test/A
> - B", name_len=22, context=0x7f9cac001000) at lookup_hosts.c:403
> #5  0x000055906a79afbd in do_lookup_mount (ap=ap@entry=0x55906c379910,
> map=map@entry=0x55906c379ae0, name=name@entry=0x7f9cb25c6d80
> "/net/rhel73/test/A - B", name_len=name_len@entry=22) at lookup.c:845
> #6  0x000055906a79bad3 in lookup_nss_mount (ap=ap@entry=0x55906c379910,
> source=source@entry=0x0, name=name@entry=0x7f9cb25c6d80 "/net/rhel73/test/A
> - B", name_len=22) at lookup.c:1239
> #7  0x000055906a794674 in do_mount_direct (arg=<optimized out>) at
> direct.c:1197
> #8  0x00007f9cbe1eaea5 in start_thread (arg=0x7f9cb25c8700) at
> pthread_create.c:307
> #9  0x00007f9cbce1b8dd in clone () at
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
> (gdb) f 1
> #1  0x00007f9cb8e629b0 in mount_subtree (ap=ap@entry=0x55906c379910,
> me=me@entry=0x7f9c98002660, name=name@entry=0x7f9cb25c6d80
> "/net/rhel73/test/A - B", loc=loc@entry=0x0, 
>     options=options@entry=0x7f9c98002890 "", ctxt=ctxt@entry=0x7f9cac001060)
> at parse_sun.c:1202
> 1202			int loclen = strlen(loc);
> (gdb) f 2
> #2  0x00007f9cb8e639ed in parse_mount (ap=0x55906c379910, name=<optimized
> out>, name_len=<optimized out>, mapent=<optimized out>, context=<optimized
> out>) at parse_sun.c:1545
> 1545			rv = mount_subtree(ap, me, name, NULL, options, ctxt);
> 
> 
> // since this particular entry is considered a multi-mount 
> // so we take the else on (rhel7):
> 
> 1146         if (me == me->multi) {
> ...
> 1201         } else {
> 1202                 int loclen = strlen(loc);
> 
> // but loc string is null
> 
> // as i could see, the problem is that we're mistakenly treating that map
> entry as a multi-mount itself, checking with debug output :

Yes, so maybe the implicit assumptions aren't that far wrong ...

Thanks for the information, I'll have a look at it.

> 
> Aug 16 15:45:42 rhel77 automount[4441]: handle_packet: type = 5
> Aug 16 15:45:42 rhel77 automount[4441]: handle_packet_missing_direct: token
> 7, name /net/rhel73/test/A - B, request pid 2419
> Aug 16 15:45:42 rhel77 automount[4441]: attempting to mount entry
> /net/rhel73/test/A - B
> Aug 16 15:45:42 rhel77 kernel: automount[4488]: segfault at 0 ip
> 00007f9cbce8c691 sp 00007f9cb25c66e8 error 4 in
> libc-2.17.so[7f9cbcd1d000+1c3000]
> Aug 16 15:45:42 rhel77 automount[4441]: lookup_mount: lookup(hosts):
> /net/rhel73/test/A - B -> rhel73:/test/A - B
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mount: parse(sun): expanded
> entry: rhel73:/test/A - B
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mount: parse(sun): gathered
> options:
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mount: parse(sun):
> dequote("/") -> /
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mapent: parse(sun): gathered
> options:
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mapent: parse(sun):
> dequote("rhel73:/test/A") -> rhel73:/test/A
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mapent: parse(sun):
> dequote("-") -> -
> Aug 16 15:45:42 rhel77 automount[4441]: parse_mapent: parse(sun):
> dequote("B") -> B
> Aug 16 15:45:42 rhel77 automount[4441]: update_offset_entry: parse(sun):
> updated multi-mount offset / -> rhel73:/test/A - B  <<--- ?
> 
> Aug 16 15:45:47 rhel77 systemd-coredump[4489]: Process 4441 (automount) of
> user 0 dumped core.
>                                                #4  0x000055906a79afbd
> do_lookup_mount (automount)
>                                                #5  0x000055906a79bad3
> lookup_nss_mount (automount)
>                                                #6  0x000055906a794674
> do_mount_direct (automount)
>                                                #1  0x000055906a79d743
> st_queue_handler (automount)
>                                                #1  0x000055906a7a8f89
> alarm_handler (automount)
>                                                #1  0x000055906a78eb76
> statemachine (automount)
>                                                #3  0x000055906a78f1e5 _start
> (automount)
> 
> // on check_is_multi() 
> 
> 740 static int check_is_multi(const char *mapent)
>  741 {
>  742         const char *p = mapent;
>  743         int multi = 0;
>  744         int not_first_chunk = 0;
>  745 
>  746         if (!p) {
>  747                 logerr(MODPREFIX "unexpected NULL map entry pointer");
>  748                 return 0;
>  749         }
>  750 
>  751         if (*p == '"')
>  752                 p++;
>  753 
>  754         /* If first character is "/" it's a multi-mount */
>  755         if (*p == '/')
>  756                 return 1;
>  757 
>  758         while (*p) {
>  759                 p = skipspace(p);
> ...
>  768                  */
>  769                 if (not_first_chunk) {
>  770                         if (*p == '"')
>  771                                 p++;
>  772                         if (*p == '/' || *p == '-') {  <<<----  here we
> are on this part of the string : rhel73:/test/A - B
>  773                                 multi = 1;                             
> ^
>  774                                 break;
>  775                         }
>  776                 }
> 
> // not sure why we check for "-" here. Maybe because multi-mount entry can
> also contain a buit-in map ???

Possibly but this has been around for a while and there have been some
unusual special cases so I probably won't be able to work out why it's
there.

> // does this comply with SUN auto map format ?

I do try and stick with Sun map format everywhere, at least most of the
time.

Truth is I have wanted to convert this map parser to use a yacc like
generated parser (say bison) for a long time now but when I have tried
it has proven quite difficult so hasn't been done yet.