Anant Thazhemadam wrote on Mon, Oct 12, 2020: > You mentioned how a fully zeroed address isn't exactly faulty. By extension, > wouldn't that > mean that an address that simply begins with a 0 isn't faulty as well?
That is correct. If you have a look at the unix(7) man page that describes AF_UNIX, it describes what 'abstract' addresses are and unix_mkname() in linux's net/unix/af_unix.c shows how it's handled. > This is an interesting point, because if the condition is modified to > checking for addr[0] directly, > addresses that simply begin with 0 (but have more non-zero content following) > wouldn't be > copied over either, right? Yes, we would reject any address that starts with a nul byte -- but that is already exactly what your patch does with strlen() already: a '\0' at the start of the string is equivalent to strlen(addr) == 0. The only difference is that checking for addr[0] won't run through all the string if it doesn't start with a nul byte; but this is a one-time thing at mount so it really doesn't matter. > In the end, it comes down to what you define as a "valid" value that sun_path > can have. > We've already agreed that a fully zeroed address wouldn't qualify as a valid > value for sun_path. > Are addresses that aren't fully zeroed, but only begin with a 0 also to be > considered as an > unacceptable value for sun_path? Yes, because the strcpy() a few lines below would copy nothing, leaving sun_server.sun_path uninitialized like your example. At that point you could ask why not "fix" that strcpy to properly copy the address passed instead but that doesn't really make sense given where 'addr' comes from: it's passed from userspace as a nul-terminated string, so nothing after the first '\0' is valid. There probably are ways to work around that (e.g. iproute's ss will display abstract addresses with a leading '@' instead) but given nobody ever seemed to care I think it's safe to just return EINVAL there like you did ; there's nothing wrong with your patch as far as I'm concerned. -- Dominique
