[PATCH] libext2fs: don't flush to device opened in read-only mode

[PATCH] libext2fs: don't flush to device opened in read-only mode

[Konstantin Chistyakov]

From: Konstantin Chistyakov <Konstantin.Chistyakov@Paragon-software.com>
Date: Tue, 19 May 2017 09:23:29 -0600
Subject: [PATCH] libext2fs: don't flush to device opened in read-only mode

From: Konstantin Chistyakov <Konstantin.Chistyakov@Paragon-software.com>

If the e2fsck is called with both -f -n options, the checking completing with a flushing error (retcode 8).

Catch error from the unix_flush() fsync(data->dev) when data->dev is opened in read-only mode:

...
e2fsck -n -f /dev/sdd1
e2fsck 1.43.4 (31-Jan-2017)
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Error flushing writes to storage device: Permission denied

---
lib/ext2fs/unix_io.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib/ext2fs/unix_io.c')

diff -up lib/ext2fs/unix_io.c{.orig,}
--- lib/ext2fs/unix_io.c.orig        2017-05-19 14:32:12.550414900 +0300
+++ lib/ext2fs/unix_io.c              2017-05-19 14:33:48.520878000 +0300
@@ -1030,7 +1030,7 @@ static errcode_t unix_flush(io_channel channel)
#ifndef NO_IO_CACHE
 	retval = flush_cached_blocks(channel, data, 0);
#endif
-	if (!retval && fsync(data->dev) != 0)
+	if (!retval && (data->flags & IO_FLAG_RW) && fsync(data->dev) != 0)
 		return errno;
 	return retval;
}

Re: libext2fs: don't flush to device opened in read-only mode

[Theodore Ts'o]

On Mon, May 22, 2017 at 09:48:00AM +0000, Konstantin Chistyakov wrote:
> From: Konstantin Chistyakov <Konstantin.Chistyakov@Paragon-software.com>
> Date: Tue, 19 May 2017 09:23:29 -0600
> Subject: [PATCH] libext2fs: don't flush to device opened in read-only mode
> 
> From: Konstantin Chistyakov <Konstantin.Chistyakov@Paragon-software.com>
> 
> If the e2fsck is called with both -f -n options, the checking completing with a flushing error (retcode 8).
> 
> Catch error from the unix_flush() fsync(data->dev) when data->dev is opened in read-only mode:

I can't reproduce this problem.  What storage device or operating
system are you using?

That being said, this is probably the wrong place to fix it.  The
better fix is in e2fsck/unix.c, by adding a read-only test.

       	      	 		   	  - Ted

RE: libext2fs: don't flush to device opened in read-only mode

[Konstantin Chistyakov]

Thanks for suggestion how to fix the bug correctly. Platform for reproducing: CYGWin build (x86, x64, cygwin1.dll v.2.8.0), Windows 8.1 x64 and Windows  10 x64 (x86 not tested).

-----Original Message-----
From: Theodore Ts'o [mailto:tytso@mit.edu] 
Sent: Thursday, May 25, 2017 12:56 AM
To: Konstantin Chistyakov <Konstantin.Chistyakov@paragon-software.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: libext2fs: don't flush to device opened in read-only mode

On Mon, May 22, 2017 at 09:48:00AM +0000, Konstantin Chistyakov wrote:
> From: Konstantin Chistyakov 
> <Konstantin.Chistyakov@Paragon-software.com>
> Date: Tue, 19 May 2017 09:23:29 -0600
> Subject: [PATCH] libext2fs: don't flush to device opened in read-only 
> mode
> 
> From: Konstantin Chistyakov 
> <Konstantin.Chistyakov@Paragon-software.com>
> 
> If the e2fsck is called with both -f -n options, the checking completing with a flushing error (retcode 8).
> 
> Catch error from the unix_flush() fsync(data->dev) when data->dev is opened in read-only mode:

I can't reproduce this problem.  What storage device or operating system are you using?

That being said, this is probably the wrong place to fix it.  The better fix is in e2fsck/unix.c, by adding a read-only test.

       	      	 		   	  - Ted

RE: libext2fs: don't flush to device opened in read-only mode

[Konstantin Chistyakov]

Updating path:
Subject: [PATCH] e2fsck: don't flush to device opened in read-only mode

If the e2fsck is called with both -f -n options, the checking completing with a flushing error (retcode 8).
e2fsprogs 1.43.4, builded in CygWin (cygwin1.dll 2.8.0 x64), Windows 8.1 x64 and Windows 10 x64 (not tested on x86).
In path added read-only test for flushing block in 'e2fsck/unix.c';

Catch error from the ext2fs_flush(ctx->fs)  when ctx->fs is opened in read-only mode:

...
e2fsck -n -f /dev/sdd1
e2fsck 1.43.4 (31-Jan-2017)
Pass 1: Checking inodes, blocks, and sizes
Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information
Error flushing writes to storage device: Permission denied

---
(limited to 'e2fsck/unix.c')

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index b602553..ff96148 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1898,15 +1898,17 @@ no_journal:
 		ext2fs_mark_super_dirty(fs);
 	}
 
-	e2fsck_write_bitmaps(ctx);
-	if (fs->flags & EXT2_FLAG_DIRTY) {
-		pctx.errcode = ext2fs_flush(ctx->fs);
+	if (!(ctx->options & E2F_OPT_READONLY)) {
+		e2fsck_write_bitmaps(ctx);
+		if (fs->flags & EXT2_FLAG_DIRTY) {
+			pctx.errcode = ext2fs_flush(ctx->fs);
+			if (pctx.errcode)
+				fix_problem(ctx, PR_6_FLUSH_FILESYSTEM, &pctx);
+		}
+		pctx.errcode = io_channel_flush(ctx->fs->io);
 		if (pctx.errcode)
-			fix_problem(ctx, PR_6_FLUSH_FILESYSTEM, &pctx);
+			fix_problem(ctx, PR_6_IO_FLUSH, &pctx);
 	}
-	pctx.errcode = io_channel_flush(ctx->fs->io);
-	if (pctx.errcode)
-		fix_problem(ctx, PR_6_IO_FLUSH, &pctx);
 
 	if (was_changed) {
 		int fs_fixed = (ctx->flags & E2F_FLAG_PROBLEMS_FIXED);

-----Original Message-----
From: Konstantin Chistyakov 
Sent: Thursday, May 25, 2017 8:50 AM
To: 'Theodore Ts'o' <tytso@mit.edu>
Cc: 'linux-ext4@vger.kernel.org' <linux-ext4@vger.kernel.org>
Subject: RE: libext2fs: don't flush to device opened in read-only mode

Thanks for suggestion how to fix the bug correctly. Platform for reproducing: CYGWin build (x86, x64, cygwin1.dll v.2.8.0), Windows 8.1 x64 and Windows  10 x64 (x86 not tested).

-----Original Message-----
From: Theodore Ts'o [mailto:tytso@mit.edu]
Sent: Thursday, May 25, 2017 12:56 AM
To: Konstantin Chistyakov <Konstantin.Chistyakov@paragon-software.com>
Cc: linux-ext4@vger.kernel.org
Subject: Re: libext2fs: don't flush to device opened in read-only mode

On Mon, May 22, 2017 at 09:48:00AM +0000, Konstantin Chistyakov wrote:
> From: Konstantin Chistyakov
> <Konstantin.Chistyakov@Paragon-software.com>
> Date: Tue, 19 May 2017 09:23:29 -0600
> Subject: [PATCH] libext2fs: don't flush to device opened in read-only 
> mode
> 
> From: Konstantin Chistyakov
> <Konstantin.Chistyakov@Paragon-software.com>
> 
> If the e2fsck is called with both -f -n options, the checking completing with a flushing error (retcode 8).
> 
> Catch error from the unix_flush() fsync(data->dev) when data->dev is opened in read-only mode:

I can't reproduce this problem.  What storage device or operating system are you using?

That being said, this is probably the wrong place to fix it.  The better fix is in e2fsck/unix.c, by adding a read-only test.

       	      	 		   	  - Ted

Re: libext2fs: don't flush to device opened in read-only mode

[Theodore Ts'o]

On Thu, May 25, 2017 at 12:12:21PM +0000, Konstantin Chistyakov wrote:
> Updating path:
> Subject: [PATCH] e2fsck: don't flush to device opened in read-only mode

Thanks, applied, with some typographical fix ups.

Also, in the future please include your Developer's Certification of
Origin (aka, the Signed-Off-by)[1].  In this case the patch is obvious
enough (and I suggested it), that I'll apply it without it, but in the
future, I'd appreicate if you could include it in your patches.

[1] https://developercertificate.org/

Cheers,

					- Ted