linux-ext4 - Re: /fs/ext4/namei.c ext4_find_dest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <636e24b3-b286-dc21-48b8-0097b8782a23@jguk.org>
Date:   Thu, 7 May 2020 12:25:10 +0100
From:   Jonny Grant <jg@...k.org>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>, linux-ext4@...r.kernel.org
Subject: Re: /fs/ext4/namei.c ext4_find_dest_de()



On 05/05/2020 19:50, Andreas Dilger wrote:
> On May 5, 2020, at 12:07 PM, Jonny Grant <jg@...k.org> wrote:
>> On 04/05/2020 20:52, Theodore Y. Ts'o wrote:
>>> On Mon, May 04, 2020 at 08:38:33AM +0100, Jonny Grant wrote:
>>>>>> I noticed that mkdir() returns EEXIST if a directory already exists.
>>>>>> strerror(EEXIST) text is "File exists"
>>>>>>
>>>>>> Can ext4_find_dest_de() be amended to return EISDIR if a directory already
>>>>>> exists? This will make the error message clearer.
>>>>>
>>>>> No; this will confuse potentially a large number of existing programs.
>>>>> Also, the current behavior is required by POSIX and the Single Unix
>>>>> Specification standards.
>>>>>
>>>>> 	https://pubs.opengroup.org/onlinepubs/009695399/
>>>>>
>>>> Is it likely POSIX would introduce this change? It's a shame we're still
>>>> constrained by old standards (SVr4, BSD), but it's fine if they can be
>>>> updated.
>>> No, because it has the potential to break existing Unix/Linux/Posix-compliant
>>> programs.  There may very well be C programs doing the following....
>>> 	   if (mkdir(filename) < 0) {
>>> 	   	if (errno != EEXIST) {
>>> 			perror(filename);
>>> 			exit(1);
>>> 		}
>>> 	}
>>> For example, there may very well be implementations of "mkdir -p" that
>>> do precisely this.
>>> If we change the error returned by the mkdir system call as you
>>> propose, it would break these innocent, unsuspecting programs.  That's
>>> not something which will be allowed, because it falls into the
>>> category of a Bad Thing.
>>
>> Thank you for your reply.
>>
>> What's an appropriate solution to this problem?
>>
>> To achieve the desired output. when a directory exists.
>>
>> $ mkdir test
>> $ mkdir: cannot create directory ‘test’: Is a directory
> 
> I don't think it is reasonable to change the EEXIST return code just
> to make you happy.  However, it may be within your purview to change
> the mkdir(1) code to improve the error message:
> 
> 	rc = mkdir(name, mode);
> 	if (rc < 0) {
> 		if (errno == EEXIST)
> 			errmsg = _("File or directory already exists");
>                  else
>                          errmsg = strerror(errno);
> 		fprintf(stderr, "%s: cannot create directory '%s': %s\n",
> 			progname, name, errmsg);
>          }
> 
> or whatever you want.  If you are really keen, you could try to change
> the string that strerror(EEXIST) provides to be more generic, but that
> may also break programs that parse the output of mkdir(1) for some reason.

I doubt glibc would ever agree to change strerror(EEXIST).


> I would *not* recommend to change this to stat() the target name to
> determine the file type just to print the error message, as that is just
> useless overhead, of which there is too much in GNU fileutils already.
> 
> On the flip side, what is the driver for making this change?  The existing
> error message has been OK for users for 40 years already?

It's a pet peeve, saw it in some logs from our software, wondered if the 
message could be clear as I knew there is the EISDIR errno.

I recall someone else mentioned adding another mode flag, O_ to allow 
the kernel to return EISDIR if it knows that, that would work, then 
programs could enable it in the future.

Cheers, Jonny