Close because of:
https://reviews.freebsd.org/D35544

Close because of:
https://reviews.freebsd.org/D35544

Ok, let's ignore mangled dir entry on rename.
The situation will looks like:

root@fb:/mnt # mkdir -p DIR0/dirbad2 DIR1
root@fb:/mnt # mv ./DIR0/dirbad2 ./DIR1
root@fb:/mnt # tree
.
├── DIR0
├── DIR1
│   └── dirbad2
└── lost+found

I thought about this patch little bit more and found, that it is not so simple to not panic in all places.
First place:

	/*
	 * Check that directory length properly reflects presence
	 * of this entry.
	 */
	if (entryoffsetinblock + EXT2_DIR_REC_LEN(ep->e2d_namlen) >
	    dp->i_size) {
		ext2_dirbad(dp, i_offset, "i_size too small");
		brelse(bp);
		return (EIO);
	}
	brelse(bp);

It is possible do not panic in this case, because it is just incorrect i_size and could be simple fixed by fsck.

Ok, in case of ext2fs, we not need printf and leave only dtrace probe with error reporting.
I hope user will smart enough to unmount and run fsck, if will get errors on directory listing.

In D38503#876411, @pfg wrote:

Hmm.. we should keep consistency with UFS .. and that means panic in ext2_dirbad(). My guess is that for this case you shouldn' t call ext2_dirbad().

Ok, let's remove ext2_check_root_direntry() completely.

In D38245#869181, @kib wrote:

This change triggered me because explicit cr_uid checks mean that e.g. a future implementation of ACLs would be broken there.
VOP_ACCESS()/VOP_ACCESSX() should encapsulate such knowledge of permissions.

Also, isn't remove from sticky directory has similar problem?

The similar test for removing will pass (tests/rmdir/11.t).
Let's consider the test explanation from pjdfstest comment [https://github.com/pjd/pjdfstest/blob/master/tests/rename/09.t#L121]:

# User owns the source sticky directory, but doesn't own the source file.
# This fails when changing parent directory, because this will modify
# source directory inode (the .. link in it), but we can still rename it
# without changing its parent directory.

So, it mean, we cannot modify renamed inode, but can remove it.
Let's add some graphics:

sticky directory                child directory
owned by user (ino 100500)      not owned by user (ino 100501)
         |                              |
        \|/                            \|/
directory entries:                  directory entries: 
- "."     (ino 100500)              - "."     (ino 100501)
- ".."    (ino XXXXXX)    <-----    - ".."    (ino 100500)  <- cannot be modified
...                                 ...
- "child" (ino 100501)     ----->   ...

Changes:

• Implement extended attributes max size variable as mount option instead of sysctl, the value could be changed during remount (MNT_UPDATE)
• Misc refactoring:
  • Remove tmpfs_extattr_valid_attrname() function to avoid double strlen call.
  • The if expressions fixes.
  • Decrease number of blank lines.

Add links to testcases to source code.

This version of e2fs_first_dblock check will fail on 1k block size. The

fs->e2fs->e2fs_bcount >= 1024

is always true.

I am working on more complex extents verification logic, but let it to be committed.

The first data block value check should be more complex. See s_first_data_block superblock field here:
https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout

I think it will be better to commit another crc16 implementation and then apply 'code duplication fix' by moving function used in both places to independent header. The reason is, that I cannot confirm that it is 'generic' and 'right' crc16 implementation. I can only confirm, that this implementation is suitable for ext2fs.

Fix printf message as requested

Make functions inline

Fix and expression order.

Looks good from ext2fs side.

@kib, you mentioned in SUMMARY:
"ext2fs change to reset clusterw on truncation should be a bug fix."

Thank you for fix.

In D23259#543519, @pfg wrote:

In D23259#543516, @fsu wrote:

In D23259#543509, @pfg wrote:

Sorry I meant to approve this:
https://reviews.freebsd.org/D24685

(fsu should have comandeered this one instead of opening a new one)

I did not managed with new diff uploading, diff did not want to attach to this revision.
Seems like, I can attach diffs only to revisions, which were opened by me.
Please correct me if I am wrong or lose something in the Phabricator UI.

You had to comandeer it in the Action box. You can't do it anymore because it is closed but I know because other people have comandeered my WIP changes ;).

No problem about the new revision though ... Good Job !

In D23259#543509, @pfg wrote:

Sorry I meant to approve this:
https://reviews.freebsd.org/D24685

(fsu should have comandeered this one instead of opening a new one)

In D13209#537208, @pfg wrote:

In D13209#537057, @trasz wrote:

Ping? What's the status of this?

FWIW, I recall kib had objections and fsu needed clarifications but he never got them.
There was never urgency as there is no known program using this code so the issue has been dormant for a long time.

As I remember, the task appeared to be more difficult than it was expected by me. More changes appeared to be required to implement, than it was expected from start.

I am not sure that dinode and superblock conversion will be enough, as I checked on linux side, the group descriptors should be converted too, possible same for bitmaps.
From https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout
"All fields in ext4 are written to disk in little-endian order"