[PATCH] btrfs-progs: fix fi du so it works in more cases

[PATCH] btrfs-progs: fix fi du so it works in more cases

[Austin S. Hemmelgarn]

Currently, btrfs fi du uses open_file_or_dir(), which tries to open
it's argument with o_RDWR.  Because of POSIX semantics, this fails for
non-root users when the file is read-only or is an executable that
is being run currently, or for all users (including root) when the
filesystem is read-only.  THis results in a somewhat confusing 'Unknown
error -1' message when trying to check such files.  Switch to using
open_file_or_dir3() with O_RDONLY passed in the flags, as this avoids
the limitations listed above, and we have no need to write to the files
anyway (and thus shouldn't be opening them writable).

Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
Build and runtime tested on x86-64 with glibc.

I intend to take the time at some point this week to audit all users of
open_file_or_dir() and similarly change any that don't need to write
to what they're opening, possibly adding a helper function to do a
read-only open.

 cmds-fi-du.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-fi-du.c b/cmds-fi-du.c
index 2ffd917..168fc72 100644
--- a/cmds-fi-du.c
+++ b/cmds-fi-du.c
@@ -438,7 +438,7 @@ static int du_add_file(const char *filename, int dirfd,
 		ret = sprintf(pathp, "/%s", filename);
 	pathp += ret;
 
-	fd = open_file_or_dir(path, &dirstream);
+	fd = open_file_or_dir3(path, &dirstream, O_RDONLY);
 	if (fd < 0) {
 		ret = fd;
 		goto out;
-- 
2.7.4

Re: [PATCH] btrfs-progs: fix fi du so it works in more cases

[Martin Volf]

Works for me, thanks.

Martin Volf

On Mon, Mar 21, 2016 at 1:23 PM, Austin S. Hemmelgarn
<ahferroin7@gmail.com> wrote:
> Currently, btrfs fi du uses open_file_or_dir(), which tries to open
> it's argument with o_RDWR.  Because of POSIX semantics, this fails for
> non-root users when the file is read-only or is an executable that
> is being run currently, or for all users (including root) when the
> filesystem is read-only.  THis results in a somewhat confusing 'Unknown
> error -1' message when trying to check such files.  Switch to using
> open_file_or_dir3() with O_RDONLY passed in the flags, as this avoids
> the limitations listed above, and we have no need to write to the files
> anyway (and thus shouldn't be opening them writable).
>
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
> Build and runtime tested on x86-64 with glibc.
>
> I intend to take the time at some point this week to audit all users of
> open_file_or_dir() and similarly change any that don't need to write
> to what they're opening, possibly adding a helper function to do a
> read-only open.
>
>  cmds-fi-du.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmds-fi-du.c b/cmds-fi-du.c
> index 2ffd917..168fc72 100644
> --- a/cmds-fi-du.c
> +++ b/cmds-fi-du.c
> @@ -438,7 +438,7 @@ static int du_add_file(const char *filename, int dirfd,
>                 ret = sprintf(pathp, "/%s", filename);
>         pathp += ret;
>
> -       fd = open_file_or_dir(path, &dirstream);
> +       fd = open_file_or_dir3(path, &dirstream, O_RDONLY);
>         if (fd < 0) {
>                 ret = fd;
>                 goto out;
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Martin Volf

Re: [PATCH] btrfs-progs: fix fi du so it works in more cases

[David Sterba]

On Mon, Mar 21, 2016 at 08:23:11AM -0400, Austin S. Hemmelgarn wrote:
> Currently, btrfs fi du uses open_file_or_dir(), which tries to open
> it's argument with o_RDWR.  Because of POSIX semantics, this fails for
> non-root users when the file is read-only or is an executable that
> is being run currently, or for all users (including root) when the
> filesystem is read-only.  THis results in a somewhat confusing 'Unknown
> error -1' message when trying to check such files.  Switch to using
> open_file_or_dir3() with O_RDONLY passed in the flags, as this avoids
> the limitations listed above, and we have no need to write to the files
> anyway (and thus shouldn't be opening them writable).
> 
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>

Applied, thanks.

> ---
> Build and runtime tested on x86-64 with glibc.
> 
> I intend to take the time at some point this week to audit all users of
> open_file_or_dir() and similarly change any that don't need to write
> to what they're opening, possibly adding a helper function to do a
> read-only open.

Thanks. I think using open_file_or_dir3(path, &dirstream, O_RDONLY) is
ok, no need for a helper.

Re: [PATCH] btrfs-progs: fix fi du so it works in more cases

[Austin S. Hemmelgarn]

On 2016-03-21 13:40, David Sterba wrote:
> On Mon, Mar 21, 2016 at 08:23:11AM -0400, Austin S. Hemmelgarn wrote:
>> Currently, btrfs fi du uses open_file_or_dir(), which tries to open
>> it's argument with o_RDWR.  Because of POSIX semantics, this fails for
>> non-root users when the file is read-only or is an executable that
>> is being run currently, or for all users (including root) when the
>> filesystem is read-only.  THis results in a somewhat confusing 'Unknown
>> error -1' message when trying to check such files.  Switch to using
>> open_file_or_dir3() with O_RDONLY passed in the flags, as this avoids
>> the limitations listed above, and we have no need to write to the files
>> anyway (and thus shouldn't be opening them writable).
>>
>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>
> Applied, thanks.
>
>> ---
>> Build and runtime tested on x86-64 with glibc.
>>
>> I intend to take the time at some point this week to audit all users of
>> open_file_or_dir() and similarly change any that don't need to write
>> to what they're opening, possibly adding a helper function to do a
>> read-only open.
>
> Thanks. I think using open_file_or_dir3(path, &dirstream, O_RDONLY) is
> ok, no need for a helper.
>
Agreed, especially since this appears to be the only place that used 
open_file_or_dir() that doesn't potentially need write access.