[f2fs-dev] [Bug 200871] F2FS experiences data loss (entry is completely lost) when an I/O failure occurs.

2018-08-26 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200871

--- Comment #2 from Stathis Maneas (sman...@cs.toronto.edu) ---
1. I am using a device mapper module similar to the existing dm-flakey module:
https://elixir.bootlin.com/linux/v4.18/source/drivers/md/dm-flakey.c

Specifically, an error is introduced to an I/O operation in the same way as
implemented by the dm-flakey module
(https://elixir.bootlin.com/linux/v4.18/source/drivers/md/dm-flakey.c#L348):
...
else if (test_bit(ERROR_WRITES, &fc->flags)) {
bio_io_error(bio);
return DM_MAPIO_SUBMITTED;
}
...

In this case, fsync is not able to capture the error and returns as if no error
has taken place.

Here, I would like to mention that using this mechanism to introduce errors
when ext4 is used as the underlying file system, results in fsync returning an
error.

2. Indeed, the problem here is that the corresponding NAT table entry points to
an inode that has never been persisted on disk. However, since fsync does not
report an error, someone would hope that the entry would be there.

Moreover, the problem here is worse, because after invoking fsck, the entry
completely disappears from the file system and cannot be restored in any way.

Had fsync returned with an error, the user would have known that the operation
has failed and thus, they would not have excepted any modifications to have
taken place.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [Bug 200635] Oops error in refresh_sit_entry() while unmounting a crafted F2FS image

2018-08-26 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200635

--- Comment #5 from Stathis Maneas (sman...@cs.toronto.edu) ---
For the case where the checksum mechanism is not enabled for the superblock, it
would be nice to fix the error and make the file system able to be mounted once
again, provided that there is a safe way to restore the values stored in the
superblock using any of the underlying checkpoints.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [RFC PATCH 01/10] fs-verity: add setup code, UAPI, and Kconfig

2018-08-26 Thread Eric Biggers
Hi Chuck,

On Sun, Aug 26, 2018 at 12:22:08PM -0400, Chuck Lever wrote:
> Hi Eric-
> 
> Context: I'm working on IMA support for NFSv4, and would like to
> use fs-verity (or some Merkle tree-like mechanism) eventually to
> help address the performance impacts of using IMA with large NFS
> files.
> 
> 
> > On Aug 24, 2018, at 12:16 PM, Eric Biggers  wrote:
> > 
> > From: Eric Biggers 
> > 
> > fs-verity is a filesystem feature that provides efficient, transparent
> > integrity verification and authentication of read-only files.  It uses a
> > dm-verity like mechanism at the file level: a Merkle tree hidden past
> > the end of the file is used to verify any block in the file in
> > log(filesize) time.  It is implemented mainly by helper functions in
> > fs/verity/ that will be shared by multiple filesystems.
> 
> This description suggests that the only way fs-verity can work is
> by placing the Merkle tree data after EOF. Further, this organi-
> zation is exposed to user space, making it a fixed part of the
> fs-verity kernel/user space API.
> 
> Remote filesystems -- esp. NFS -- would prefer to manage the Merkle
> tree data in other ways. The NFSv4 protocol, for example, supports
> named streams (as some other filesystems do), and could store the
> Merkle trees in those. Or, a new pNFS layout type could be con-
> structed where Merkle trees are stored separately from a file's
> content -- perhaps even on a separate file server.
> 
> File servers can store this data as the servers' local filesystems
> require.
> 
> Sharing how the Merkle tree is created and used is sensible, but
> IMHO the filesystem implementations should be allowed to store this
> tree however they find convenient. The Merkle trees should be
> exposed via a clean API, not as part of the file's content.
> 

There has also been discussion with this on the thread for patch 02/10.
"A Merkle tree hidden past the end of the file" describes how ext4 and f2fs are
proposed to implement it, and it describes the file format expected by
FS_IOC_ENABLE_VERITY.  But, at FS_IOC_ENABLE_VERITY time, a filesystem could
copy the verity metadata to somewhere else if it wanted, e.g. into a file
stream, and then truncate the file to its original size.

Afterwards, fs-verity doesn't really care where the metadata is stored.
Currently it does actually assume it's beyond EOF since it calls
read_mapping_page() directly, but that could be replaced at any time with
indirection via a method fsverity_operations.read_metadata_page().
We actually had such a method originally, but it turned out to be unnecessary
for ext4 and f2fs, so I had dropped it for now.

I will make this clearer in the next revision of the patchset, and maybe even
consider reintroducing ->read_metadata_page() to make it clear that filesystems
don't necessarily have to store the metadata beyond EOF.

Thanks,

- Eric

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [RFC PATCH 02/10] fs-verity: add data verification hooks for ->readpages()

2018-08-26 Thread Gao Xiang via Linux-f2fs-devel
Hi Eric,

On 2018/8/27 1:04, Eric Biggers wrote:
> Hi Chuck,
> 
> On Sun, Aug 26, 2018 at 11:55:57AM -0400, Chuck Lever wrote:
>>> +
>>> +/**
>>> + * fsverity_verify_page - verify a data page
>>> + *
>>> + * Verify a page that has just been read from a file against that file's 
>>> Merkle
>>> + * tree.  The page is assumed to be a pagecache page.
>>> + *
>>> + * Return: true if the page is valid, else false.
>>> + */
>>> +bool fsverity_verify_page(struct page *data_page)
>>> +{
>>> +   struct inode *inode = data_page->mapping->host;
>>> +   const struct fsverity_info *vi = get_fsverity_info(inode);
>>> +   struct ahash_request *req;
>>> +   bool valid;
>>> +
>>> +   req = ahash_request_alloc(vi->hash_alg->tfm, GFP_KERNEL);

Some minor suggestions occurred to me after I saw this part of code
again before sleeping...

1) How about introducing an iterator callback to avoid too many
ahash_request_alloc and ahash_request_free... (It seems too many pages
and could be some slower than fsverity_verify_bio...)

2) How about adding a gfp_t input argument since I don't know whether
GFP_KERNEL is suitable for all use cases...

It seems there could be more fsverity_verify_page users as well as
fsverity_verify_bio ;)

Sorry for interruption...

Thanks,
Gao Xiang

>>> +   if (unlikely(!req))
>>> +   return false;
>>> +
>>> +   valid = verify_page(inode, vi, req, data_page);
>>> +
>>> +   ahash_request_free(req);
>>> +
>>> +   return valid;
>>> +}
>>> +EXPORT_SYMBOL_GPL(fsverity_verify_page);
>>> +
>>> +/**
>>> + * fsverity_verify_bio - verify a 'read' bio that has just completed
>>> + *
>>> + * Verify a set of pages that have just been read from a file against that
>>> + * file's Merkle tree.  The pages are assumed to be pagecache pages.  
>>> Pages that
>>> + * fail verification are set to the Error state.  Verification is skipped 
>>> for
>>> + * pages already in the Error state, e.g. due to fscrypt decryption 
>>> failure.
>>> + */
>>> +void fsverity_verify_bio(struct bio *bio)
>>
>> Hi Eric-
>>
>> This kind of API won't work for remote filesystems, which do not use
>> "struct bio" to do their I/O. Could a remote filesystem solely use
>> fsverity_verify_page instead?
>>
> 
> Yes, filesystems don't have to use fsverity_verify_bio().  They can call
> fsverity_verify_page() on each page instead.  I will clarify this in the next
> revision of the patchset.
> 
> - Eric
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [RFC PATCH 10/10] f2fs: fs-verity support

2018-08-26 Thread Eric Biggers
Hi Chao,

On Sat, Aug 25, 2018 at 01:54:08PM +0800, Chao Yu wrote:
> On 2018/8/25 0:16, Eric Biggers wrote:
> > From: Eric Biggers 
> >  #ifdef CONFIG_F2FS_CHECK_FS
> >  #define f2fs_bug_on(sbi, condition)BUG_ON(condition)
> >  #else
> > @@ -146,7 +149,7 @@ struct f2fs_mount_info {
> >  #define F2FS_FEATURE_QUOTA_INO 0x0080
> >  #define F2FS_FEATURE_INODE_CRTIME  0x0100
> >  #define F2FS_FEATURE_LOST_FOUND0x0200
> > -#define F2FS_FEATURE_VERITY0x0400  /* reserved */
> > +#define F2FS_FEATURE_VERITY0x0400
> >  
> >  #define F2FS_HAS_FEATURE(sb, mask) \
> > ((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
> > @@ -598,7 +601,7 @@ enum {
> >  #define FADVISE_ENC_NAME_BIT   0x08
> >  #define FADVISE_KEEP_SIZE_BIT  0x10
> >  #define FADVISE_HOT_BIT0x20
> > -#define FADVISE_VERITY_BIT 0x40/* reserved */
> > +#define FADVISE_VERITY_BIT 0x40
> 
> As I suggested before, how about moving f2fs' verity_bit from i_fadvise to 
> more
> generic i_flags field like ext4, so we can a) remaining more bits for those
> demands which really need file advise fields. b) using i_flags bits keeping 
> line
> with ext4. Not sure, if user want to know whether the file is verity one, it
> will be easy for f2fs to export the status through FS_IOC_SETFLAGS.
> 
> #define EXT4_VERITY_FL0x0010 /* Verity protected 
> inode */
> 
> #define F2FS_VERITY_FL0x0010 /* Verity protected 
> inode */
> 

I don't like using i_advise much either, but I actually don't see either
location being much better than the other at the moment.  The real problem is an
artificial one: the i_flags in f2fs's on-disk format are being assumed to use
the same numbering scheme as ext4's on-disk format, which makes it seem that
they have to be in sync, and that all new ext4 flags (say, EA_INODE) also
reserve bits in f2fs and vice versa, when they in fact do not.  Instead, f2fs
should use its own numbering for its i_flags, and it should map them to/from
whatever is needed for common APIs like FS_IOC_{GET,SET}FLAGS and
FS_IOC_FS{GET,SET}XATTR.

So putting the verity flag in *either* location (i_advise or i_flags) is just
kicking the can down the road.  If I get around to it I will send a patch that
cleans up the f2fs flags properly...

Thanks,

- Eric

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [RFC PATCH 02/10] fs-verity: add data verification hooks for ->readpages()

2018-08-26 Thread Chuck Lever



> On Aug 24, 2018, at 12:16 PM, Eric Biggers  wrote:
> 
> From: Eric Biggers 
> 
> Add functions that verify data pages that have been read from a
> fs-verity file, against that file's Merkle tree.  These will be called
> from filesystems' ->readpage() and ->readpages() methods.
> 
> Since data verification can block, a workqueue is provided for these
> methods to enqueue verification work from their bio completion callback.
> 
> Signed-off-by: Eric Biggers 
> ---
> fs/verity/Makefile   |   2 +-
> fs/verity/fsverity_private.h |   3 +
> fs/verity/setup.c|  26 ++-
> fs/verity/verify.c   | 310 +++
> include/linux/fsverity.h |  23 +++
> 5 files changed, 362 insertions(+), 2 deletions(-)
> create mode 100644 fs/verity/verify.c
> 
> diff --git a/fs/verity/Makefile b/fs/verity/Makefile
> index 39e123805c827..a6c7cefb61ab7 100644
> --- a/fs/verity/Makefile
> +++ b/fs/verity/Makefile
> @@ -1,3 +1,3 @@
> obj-$(CONFIG_FS_VERITY)   += fsverity.o
> 
> -fsverity-y := hash_algs.o setup.o
> +fsverity-y := hash_algs.o setup.o verify.o
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index a18ff645695f4..c553f99dc4973 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -96,4 +96,7 @@ static inline bool set_fsverity_info(struct inode *inode,
>   return true;
> }
> 
> +/* verify.c */
> +extern struct workqueue_struct *fsverity_read_workqueue;
> +
> #endif /* _FSVERITY_PRIVATE_H */
> diff --git a/fs/verity/setup.c b/fs/verity/setup.c
> index e675c52898d5b..84cc2edeca25b 100644
> --- a/fs/verity/setup.c
> +++ b/fs/verity/setup.c
> @@ -824,18 +824,42 @@ EXPORT_SYMBOL_GPL(fsverity_full_i_size);
> 
> static int __init fsverity_module_init(void)
> {
> + int err;
> +
> + /*
> +  * Use an unbound workqueue to allow bios to be verified in parallel
> +  * even when they happen to complete on the same CPU.  This sacrifices
> +  * locality, but it's worthwhile since hashing is CPU-intensive.
> +  *
> +  * Also use a high-priority workqueue to prioritize verification work,
> +  * which blocks reads from completing, over regular application tasks.
> +  */
> + err = -ENOMEM;
> + fsverity_read_workqueue = alloc_workqueue("fsverity_read_queue",
> +   WQ_UNBOUND | WQ_HIGHPRI,
> +   num_online_cpus());
> + if (!fsverity_read_workqueue)
> + goto error;
> +
> + err = -ENOMEM;
>   fsverity_info_cachep = KMEM_CACHE(fsverity_info, SLAB_RECLAIM_ACCOUNT);
>   if (!fsverity_info_cachep)
> - return -ENOMEM;
> + goto error_free_workqueue;
> 
>   fsverity_check_hash_algs();
> 
>   pr_debug("Initialized fs-verity\n");
>   return 0;
> +
> +error_free_workqueue:
> + destroy_workqueue(fsverity_read_workqueue);
> +error:
> + return err;
> }
> 
> static void __exit fsverity_module_exit(void)
> {
> + destroy_workqueue(fsverity_read_workqueue);
>   kmem_cache_destroy(fsverity_info_cachep);
>   fsverity_exit_hash_algs();
> }
> diff --git a/fs/verity/verify.c b/fs/verity/verify.c
> new file mode 100644
> index 0..1452dd05f75d3
> --- /dev/null
> +++ b/fs/verity/verify.c
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * fs/verity/verify.c: fs-verity data verification functions,
> + *  i.e. hooks for ->readpages()
> + *
> + * Copyright (C) 2018 Google LLC
> + *
> + * Originally written by Jaegeuk Kim and Michael Halcrow;
> + * heavily rewritten by Eric Biggers.
> + */
> +
> +#include "fsverity_private.h"
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct workqueue_struct *fsverity_read_workqueue;
> +
> +/**
> + * hash_at_level() - compute the location of the block's hash at the given 
> level
> + *
> + * @vi:  (in) the file's verity info
> + * @dindex:  (in) the index of the data block being verified
> + * @level:   (in) the level of hash we want
> + * @hindex:  (out) the index of the hash block containing the wanted hash
> + * @hoffset: (out) the byte offset to the wanted hash within the hash block
> + */
> +static void hash_at_level(const struct fsverity_info *vi, pgoff_t dindex,
> +   unsigned int level, pgoff_t *hindex,
> +   unsigned int *hoffset)
> +{
> + pgoff_t hoffset_in_lvl;
> +
> + /*
> +  * Compute the offset of the hash within the level's region, in hashes.
> +  * For example, with 4096-byte blocks and 32-byte hashes, there are
> +  * 4096/32 = 128 = 2^7 hashes per hash block, i.e. log_arity = 7.  Then,
> +  * if the data block index is 65668 and we want the level 1 hash, it is
> +  * located at 65668 >> 7 = 513 hashes into the level 1 region.
> +  */
> + hoffset_in_lvl = dindex >> (level * vi->log_arity);
> +
> + /*
> +  * Compute the

Re: [f2fs-dev] [RFC PATCH 02/10] fs-verity: add data verification hooks for ->readpages()

2018-08-26 Thread Eric Biggers
Hi Chuck,

On Sun, Aug 26, 2018 at 11:55:57AM -0400, Chuck Lever wrote:
> > +
> > +/**
> > + * fsverity_verify_page - verify a data page
> > + *
> > + * Verify a page that has just been read from a file against that file's 
> > Merkle
> > + * tree.  The page is assumed to be a pagecache page.
> > + *
> > + * Return: true if the page is valid, else false.
> > + */
> > +bool fsverity_verify_page(struct page *data_page)
> > +{
> > +   struct inode *inode = data_page->mapping->host;
> > +   const struct fsverity_info *vi = get_fsverity_info(inode);
> > +   struct ahash_request *req;
> > +   bool valid;
> > +
> > +   req = ahash_request_alloc(vi->hash_alg->tfm, GFP_KERNEL);
> > +   if (unlikely(!req))
> > +   return false;
> > +
> > +   valid = verify_page(inode, vi, req, data_page);
> > +
> > +   ahash_request_free(req);
> > +
> > +   return valid;
> > +}
> > +EXPORT_SYMBOL_GPL(fsverity_verify_page);
> > +
> > +/**
> > + * fsverity_verify_bio - verify a 'read' bio that has just completed
> > + *
> > + * Verify a set of pages that have just been read from a file against that
> > + * file's Merkle tree.  The pages are assumed to be pagecache pages.  
> > Pages that
> > + * fail verification are set to the Error state.  Verification is skipped 
> > for
> > + * pages already in the Error state, e.g. due to fscrypt decryption 
> > failure.
> > + */
> > +void fsverity_verify_bio(struct bio *bio)
> 
> Hi Eric-
> 
> This kind of API won't work for remote filesystems, which do not use
> "struct bio" to do their I/O. Could a remote filesystem solely use
> fsverity_verify_page instead?
> 

Yes, filesystems don't have to use fsverity_verify_bio().  They can call
fsverity_verify_page() on each page instead.  I will clarify this in the next
revision of the patchset.

- Eric

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [RFC PATCH 01/10] fs-verity: add setup code, UAPI, and Kconfig

2018-08-26 Thread Chuck Lever
Hi Eric-

Context: I'm working on IMA support for NFSv4, and would like to
use fs-verity (or some Merkle tree-like mechanism) eventually to
help address the performance impacts of using IMA with large NFS
files.


> On Aug 24, 2018, at 12:16 PM, Eric Biggers  wrote:
> 
> From: Eric Biggers 
> 
> fs-verity is a filesystem feature that provides efficient, transparent
> integrity verification and authentication of read-only files.  It uses a
> dm-verity like mechanism at the file level: a Merkle tree hidden past
> the end of the file is used to verify any block in the file in
> log(filesize) time.  It is implemented mainly by helper functions in
> fs/verity/ that will be shared by multiple filesystems.

This description suggests that the only way fs-verity can work is
by placing the Merkle tree data after EOF. Further, this organi-
zation is exposed to user space, making it a fixed part of the
fs-verity kernel/user space API.

Remote filesystems -- esp. NFS -- would prefer to manage the Merkle
tree data in other ways. The NFSv4 protocol, for example, supports
named streams (as some other filesystems do), and could store the
Merkle trees in those. Or, a new pNFS layout type could be con-
structed where Merkle trees are stored separately from a file's
content -- perhaps even on a separate file server.

File servers can store this data as the servers' local filesystems
require.

Sharing how the Merkle tree is created and used is sensible, but
IMHO the filesystem implementations should be allowed to store this
tree however they find convenient. The Merkle trees should be
exposed via a clean API, not as part of the file's content.


> Essentially, fs-verity reports a file's hash in constant time, but reads
> that would violate that hash fail at runtime.  This is useful when only
> a portion of the file is actually accessed, as only the accessed portion
> has to be hashed, and the latency to the first read is much reduced over
> a full file hash.  On top of this hashing mechanism, auditing or
> authentication policies can be implemented to log or verify file hashes.
> 
> Note that in general, fs-verity is *not* a replacement for IMA.
> fs-verity is a lower-level feature, primarily a way to hash a file;
> whereas IMA deals more with higher-level policy logic, like defining
> which files are "measured" and what to do with those measurements.  We
> plan for IMA to support fs-verity measurements as an alternative to the
> traditional full file hash.  Still, some users find fs-verity useful by
> itself, so it's also usable without IMA in simple cases, e.g. in cases
> where just retrieving the file measurement via an ioctl is enough.
> 
> A structure containing the properties of the Merkle tree -- such as the
> hash algorithm used, the block size, and the root hash -- is also stored
> on-disk, following the Merkle tree.  The actual file measurement hash
> that fs-verity reports is the hash of this structure.
> 
> All fs-verity metadata is written by userspace; the kernel only reads
> it.  Extended attributes aren't used because the Merkle tree may be much
> larger than XATTR_SIZE_MAX, we want the hash pages to be cached in the
> page cache as usual, and in the case of fs-verity combined with fscrypt
> we want the metadata to be encrypted to avoid leaking plaintext hashes.
> The fs-verity metadata is hidden from userspace by overriding the i_size
> of the in-memory VFS inode; ext4 additionally will override the on-disk
> i_size in order to make verity a RO_COMPAT filesystem feature.
> 
> This initial patch only adds the fs-verity Kconfig option, UAPI, and
> setup code, e.g. the ->open() hook that parses the fs-verity descriptor.
> The actual ->readpages() data verification, the ioctls, ext4 and f2fs
> support, and other functionality comes in later patches.
> 
> Signed-off-by: Eric Biggers 
> ---
> fs/Kconfig|   2 +
> fs/Makefile   |   1 +
> fs/verity/Kconfig |  36 ++
> fs/verity/Makefile|   3 +
> fs/verity/fsverity_private.h  |  99 
> fs/verity/hash_algs.c | 106 +
> fs/verity/setup.c | 846 ++
> include/linux/fs.h|   9 +
> include/linux/fsverity.h  |  62 +++
> include/uapi/linux/fsverity.h |  86 
> 10 files changed, 1250 insertions(+)
> create mode 100644 fs/verity/Kconfig
> create mode 100644 fs/verity/Makefile
> create mode 100644 fs/verity/fsverity_private.h
> create mode 100644 fs/verity/hash_algs.c
> create mode 100644 fs/verity/setup.c
> create mode 100644 include/linux/fsverity.h
> create mode 100644 include/uapi/linux/fsverity.h
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index ac474a61be379..ddadc4e999429 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -105,6 +105,8 @@ config MANDATORY_FILE_LOCKING
> 
> source "fs/crypto/Kconfig"
> 
> +source "fs/verity/Kconfig"
> +
> source "fs/notify/Kconfig"
> 
> source "fs/quota/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index 

Re: [f2fs-dev] [RFC PATCH 02/10] fs-verity: add data verification hooks for ->readpages()

2018-08-26 Thread Gao Xiang via Linux-f2fs-devel
Hi Ted,

Sorry for the late reply...

On 2018/8/26 1:06, Theodore Y. Ts'o wrote:
> On Sat, Aug 25, 2018 at 03:43:43PM +0800, Gao Xiang wrote:
>>> I don't know of any plan to use fs-verity on Android's system partition or 
>>> to
>>> replace dm-verity on the system partition.  The use cases so far have been
>>> verifying files on /data, like APK files.
>>>
>>> So I don't think you need to support fs-verity in EROFS.
>>
>> Thanks for your information about fs-verity, that is quite useful for us
>> Actually, I was worrying about that these months...  :)
> 
> I'll be even clearer --- I can't *imagine* any situation where it
> would make sense to use fs-verity on the Android system partition.
> Remember, for OTA to work the system image has to be bit-for-bit
> identical to the official golden image for that release.  So the
> system image has to be completely locked down from any modification
> (to data or metadata), and that means dm-verity and *NOT* fs-verity.

I think so mainly because of the security reason you said above.

In addition, I think it is mandatory that the Android system partition
should also _never_ suffer from filesystem corrupted by design (expect
for the storage device corrupt or malware), therefore I think the
bit-for-bit read-only, and identical-verity requirement is quite strong
for Android, which will make the Android system steady and as solid as
rocks.

But I need to make sure my personal thoughts through this topic. :)

> 
> The initial use of fs-verity (as you can see if you look at AOSP) will
> be to protect a small number of privileged APK's that are stored on
> the data partition.  Previously, they were verified when they were
> downloaded, and never again.
> 
> Part of the goal which we are trying to achieve here is that even if
> the kernel gets compromised by a 0-day, a successful reboot should
> restore the system to a known state.  That is, the secure bootloader
> checks the signature of the kernel, and then in turn, dm-verity will
> verify the root Merkle hash protecting the system partition, and
> fs-verity will protect the privileged APK's.  If malware modifies any
> these components in an attempt to be persistent, the modifications
> would be detected, and the worst it could do is to cause subsequent
> reboots to fail until the phone's software could be reflashed.
> 

Yeah, I have seen the the fs-verity presentation and materials from
Android bootcamp and other official channels before.


Thanks for your kindly detailed explanation. :)


Best regards,
Gao Xiang

> Cheers,
> 
>   - Ted
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [Bug 200871] F2FS experiences data loss (entry is completely lost) when an I/O failure occurs.

2018-08-26 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=200871

Chao Yu (c...@kernel.org) changed:

   What|Removed |Added

 Status|NEW |ASSIGNED

--- Comment #1 from Chao Yu (c...@kernel.org) ---
I think there are two problems:

1. you made IO error, and return the error to upper, but fsync() didn't fail.

To reproduce this:
a) I add below code in f2fs_write_end_io, and use fault_injection to simulate
error under block layer:

if (time_to_inject(F2FS_P_SB(bio_first_page_all(bio)), FAULT_IO)) {
f2fs_show_injection_info(FAULT_IO);
bio->bi_status = BLK_STS_IOERR;
}

b) xfs_io -f /mnt/f2fs/file -c "pwrite 0 4k" -c "fsync"
fsync: Input/output error

This is because, in f2fs_write_end_io(), if error is injected, -EIO will be set
into node inode's page mapping as below:
if (unlikely(bio->bi_status)) {
mapping_set_error(page->mapping, -EIO);
if (type == F2FS_WB_CP_DATA)
f2fs_stop_checkpoint(sbi, true);
}
And later filemap_check_errors() in f2fs_sync_file() will capture such error,
and propagate it to user.

So how you inject error in bio? by fail_make_request?

2. image became inconsistent, so that we can index node block of inlien_file,
but could not read detail info of it due to the node block is corrupted.

"inconsistent node block, nid:4,
node_footer[nid:0,ino:0,ofs:0,cpver:0,blkaddr:0]"

I think this is related to first problem.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel