linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
@ 2020-08-14 12:29 Konstantin Komarov
  2020-08-14 13:17 ` Nikolay Borisov
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-14 12:29 UTC (permalink / raw)
  To: viro, linux-kernel, linux-fsdevel

This patch adds NTFS Read-Write driver to fs/ntfs3.

Having decades of expertise in commercial file systems development and huge
test coverage, we at Paragon Software GmbH want to make our contribution to
the Open Source Community by providing implementation of NTFS Read-Write
driver for the Linux Kernel.

This is fully functional NTFS Read-Write driver. Current version works with
NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.

We plan to support this version after the codebase once merged, and add new
features and fix bugs. For example, full journaling support over JBD will be
added in later updates.

The patch is too big to handle it within an e-mail body, so it is available to download 
on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
@ 2020-08-14 13:17 ` Nikolay Borisov
  2020-08-14 13:40 ` David Sterba
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2020-08-14 13:17 UTC (permalink / raw)
  To: Konstantin Komarov, viro, linux-kernel, linux-fsdevel



On 14.08.20 г. 15:29 ч., Konstantin Komarov wrote:
> This patch adds NTFS Read-Write driver to fs/ntfs3.
> 
> Having decades of expertise in commercial file systems development and huge
> test coverage, we at Paragon Software GmbH want to make our contribution to
> the Open Source Community by providing implementation of NTFS Read-Write
> driver for the Linux Kernel.
> 
> This is fully functional NTFS Read-Write driver. Current version works with
> NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.
> 
> We plan to support this version after the codebase once merged, and add new
> features and fix bugs. For example, full journaling support over JBD will be
> added in later updates.
> 
> The patch is too big to handle it within an e-mail body, so it is available to download 
> on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch
> 
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> 

So how exactly do you expect someone to review this monstrosity ?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
  2020-08-14 13:17 ` Nikolay Borisov
@ 2020-08-14 13:40 ` David Sterba
  2020-08-20  9:26   ` Konstantin Komarov
  2020-08-20 10:59   ` Konstantin Komarov
  2020-08-14 14:08 ` Aurélien Aptel
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2020-08-14 13:40 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: viro, linux-kernel, linux-fsdevel

On Fri, Aug 14, 2020 at 12:29:01PM +0000, Konstantin Komarov wrote:
> This patch adds NTFS Read-Write driver to fs/ntfs3.
> 
> Having decades of expertise in commercial file systems development and huge
> test coverage, we at Paragon Software GmbH want to make our contribution to
> the Open Source Community by providing implementation of NTFS Read-Write
> driver for the Linux Kernel.
> 
> This is fully functional NTFS Read-Write driver. Current version works with
> NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.
> 
> We plan to support this version after the codebase once merged, and add new
> features and fix bugs. For example, full journaling support over JBD will be
> added in later updates.
> 
> The patch is too big to handle it within an e-mail body, so it is available to download 
> on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch
> 
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

In case somebody wants to compile it, this fixup is needed to let 'make
fs/ntfs3/' actually work, besides enabling it in the config.

diff --git a/fs/Makefile b/fs/Makefile
index 1c7b0e3f6daa..b0b4ad8affa0 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_SYSV_FS)		+= sysv/
 obj-$(CONFIG_CIFS)		+= cifs/
 obj-$(CONFIG_HPFS_FS)		+= hpfs/
 obj-$(CONFIG_NTFS_FS)		+= ntfs/
+obj-$(CONFIG_NTFS3_FS)		+= ntfs3/
 obj-$(CONFIG_UFS_FS)		+= ufs/
 obj-$(CONFIG_EFS_FS)		+= efs/
 obj-$(CONFIG_JFFS2_FS)		+= jffs2/
diff --git a/fs/ntfs3/Makefile b/fs/ntfs3/Makefile
index 4d4fe198b8b8..d99dd1af43aa 100644
--- a/fs/ntfs3/Makefile
+++ b/fs/ntfs3/Makefile
@@ -5,7 +5,7 @@
 
 obj-$(CONFIG_NTFS3_FS) += ntfs3.o
 
-ntfs3-objs := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
+ntfs3-y := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
 	    index.o attrlist.o record.o attrib.o run.o xattr.o\
 	    upcase.o super.o file.o dir.o namei.o lznt.o\
 	    fslog.o
---

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
  2020-08-14 13:17 ` Nikolay Borisov
  2020-08-14 13:40 ` David Sterba
@ 2020-08-14 14:08 ` Aurélien Aptel
  2020-08-20 10:20   ` Konstantin Komarov
  2020-08-14 15:30 ` Aurélien Aptel
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Aurélien Aptel @ 2020-08-14 14:08 UTC (permalink / raw)
  To: Konstantin Komarov, viro, linux-kernel, linux-fsdevel

Hi Konstantin,

That's cool :) As Nikolay said it needs a little change to the makefiles
to even build.

Are you also going to publish your own mkfs.ntfs3 tool? I dont think the
existing one would support 64k clusters.

I would recommend to run checkpatch (I see already 87 warnings... some
of it is noise):

  $ ./scripts/checkpatch.pl <patch>

And sparse (I dont see much):

  $ touch fs/ntfs3/*.[ch] && make C=1

You need a recent build of sparse to do that last one. You can pass your
own sparse bin (make CHECK=~/prog/sparse/sparse C=1)

This will be a good first step.

Have you tried to run the xfstests suite against it?

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
                   ` (2 preceding siblings ...)
  2020-08-14 14:08 ` Aurélien Aptel
@ 2020-08-14 15:30 ` Aurélien Aptel
  2020-08-20 10:38   ` Konstantin Komarov
  2020-08-15 19:06 ` David Sterba
  2020-08-16  7:56 ` Pali Rohár
  5 siblings, 1 reply; 13+ messages in thread
From: Aurélien Aptel @ 2020-08-14 15:30 UTC (permalink / raw)
  To: Konstantin Komarov, viro, linux-kernel, linux-fsdevel

I've tried this using libntfs-3g mkfs.ntfs

# mkfs.ntfs /dev/vb1
# mount -t ntfs3 /dev/vb1 /mnt

This already triggered UBSAN:

 ================================================================================
 UBSAN: object-size-mismatch in fs/ntfs3/super.c:834:16
 load of address 000000006ae096b5 with insufficient space
 for an object of type 'const char'
 CPU: 3 PID: 1248 Comm: mount Not tainted 5.8.0+ #4
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4-rebuilt.opensuse.org 04/01/2014
 Call Trace:
  dump_stack+0x78/0xa0
  ubsan_epilogue+0x5/0x40
  ubsan_type_mismatch_common.cold+0xc8/0xcd
  __ubsan_handle_type_mismatch_v1+0x32/0x37
  ntfs_fill_super+0x9f/0x13e0
  ? vsnprintf+0x1ef/0x4f0
  mount_bdev+0x193/0x1c0

Which points to:

	sb->s_magic = *(unsigned long *)s_magic; /* TODO */

Maybe store ('n'<<32)|('t'<<24)|('f'<<16)|('s'<<8) ?
Seems harmless.

* * *

Then I've tried to copy /etc into it:

# cp -rp /etc /mnt

But this triggered a NULL ptr deref:

 BUG: kernel NULL pointer dereference, address: 0000000000000028
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0 
 Oops: 0000 [#1] SMP NOPTI
 CPU: 0 PID: 1255 Comm: cp Not tainted 5.8.0+ #4
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4-rebuilt.opensuse.org 04/01/2014
 RIP: 0010:ntfs_insert_security+0x187/0x4a0
 Code: 00 48 83 c4 18 85 c0 0f 85 54 01 00 00 48 8b 44 24 50 49 8d b5 d8 01 00 00 8b 54 24 60 83 c3 14 48 89 74 24 30 48 85 c0 74 3a <39> 58 28 0f 84 7e 01 00 00 49 89 e8 48 8d 4c 24 50 4c 89 f2 4c 89
 RSP: 0018:ffffac73403dfc58 EFLAGS: 00010246
 RAX: 0000000000000000 RBX: 0000000000000064 RCX: 0000000000000010
 RDX: 00000000000000b0 RSI: 0000000000000000 RDI: 0000000000000000
 RBP: ffff94154ed5fe00 R08: 0000000000000000 R09: 0000000000000001
 R10: ffff9415781a6180 R11: 0000000000000003 R12: ffff94155c989800
 R13: ffff94151e8d2a38 R14: ffff9415781a6170 R15: ffff9415781173f0
 FS:  00007fd19b86e580(0000) GS:ffff94157dc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000028 CR3: 000000001ac2a000 CR4: 0000000000350ef0
 Call Trace:
  ? mark_held_locks+0x49/0x70
  ? lockdep_hardirqs_on_prepare+0xf7/0x190
  ? ktime_get_coarse_real_ts64+0x9e/0xd0
  ? trace_hardirqs_on+0x1c/0xe0
  ntfs_create_inode+0x2db/0x11c0
  ntfs_mkdir+0x58/0x90
  vfs_mkdir+0x109/0x1f0
  do_mkdirat+0x81/0x120
  do_syscall_64+0x33/0x40
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7fd19ad54dd7
 Code: 00 b8 ff ff ff ff c3 0f 1f 40 00 48 8b 05 b9 70 2c 00 64 c7 00 5f 00 00 00 b8 ff ff ff ff c3 0f 1f 40 00 b8 53 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 91 70 2c 00 f7 d8 64 89 01 48
 RSP: 002b:00007ffec3c41588 EFLAGS: 00000206 ORIG_RAX: 0000000000000053
 RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007fd19ad54dd7
 RDX: 00000000000c0001 RSI: 00000000000001c0 RDI: 000055cad585fcf0
 RBP: 00007ffec3c41990 R08: 00007ffec3c41b50 R09: 00007fd19ada55c0
 R10: 00000000000001ef R11: 0000000000000206 R12: 00007ffec3c41b50
 R13: 0000000000000000 R14: 0000000000000000 R15: 00007ffec3c437be


(gdb) list *(ntfs_insert_security+0x187)
0xffffffff814e5097 is in ntfs_insert_security (fs/ntfs3/fsntfs.c:1811).
1806
1807            if (!e)
1808                    goto insert_security;
1809
1810    next_security:
1811            if (le32_to_cpu(e->sec_hdr.size) != new_sec_size)
1812                    goto skip_read_sds;
1813
1814            err = ntfs_read_run_nb(sbi, &ni->file.run, le64_to_cpu(e->sec_hdr.off),
1815                                   d_security, new_sec_size, NULL);

(gdb) disas /s ntfs_insert_security
....
1811            if (le32_to_cpu(e->sec_hdr.size) != new_sec_size)
   0xffffffff814e5097 <+391>:   cmp    %ebx,0x28(%rax) <=====
   0xffffffff814e509a <+394>:   je     0xffffffff814e521e <ntfs_insert_security+782>

(gdb) p/x (int)&((NTFS_DE_SDH*)0)->sec_hdr.size
$4 = 0x28

So I think 'e' is NULL. Not sure how it can happen.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
                   ` (3 preceding siblings ...)
  2020-08-14 15:30 ` Aurélien Aptel
@ 2020-08-15 19:06 ` David Sterba
  2020-08-16  0:42   ` Matthew Wilcox
  2020-08-20 10:48   ` Konstantin Komarov
  2020-08-16  7:56 ` Pali Rohár
  5 siblings, 2 replies; 13+ messages in thread
From: David Sterba @ 2020-08-15 19:06 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: viro, linux-kernel, linux-fsdevel

On Fri, Aug 14, 2020 at 12:29:01PM +0000, Konstantin Komarov wrote:
> This patch adds NTFS Read-Write driver to fs/ntfs3.
> 
> Having decades of expertise in commercial file systems development and huge
> test coverage, we at Paragon Software GmbH want to make our contribution to
> the Open Source Community by providing implementation of NTFS Read-Write
> driver for the Linux Kernel.
> 
> This is fully functional NTFS Read-Write driver. Current version works with
> NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.
> 
> We plan to support this version after the codebase once merged, and add new
> features and fix bugs. For example, full journaling support over JBD will be
> added in later updates.
> 
> The patch is too big to handle it within an e-mail body, so it is available to download 
> on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch

The way you submit the driver does not meet significant number of
requirements documented in https://www.kernel.org/doc/html/latest/process/submitting-patches.html .
so this may lead to ignoring the patch as this puts the burden on the
kernel community to make the merge somehow happen. I don't see kernel
involvement from Paragon, so let me build our half of the bridge.

As I reckon, there is interest to have NTFS implementation that's better
than the existing FUSE based support by NTFS-3G (namely performance), so
let me propose something that might lead to merging the patch
eventually.

1. check existing support in kernel

There is fs/ntfs with read-only support from Tuxera, last change in the
git tree is 3 years ago. The driver lacks read-write support so there's
not 100% functionality duplication but there's still driver for the same
filesystem.

I don't know what's the feature parity, how much the in-kernel driver is
used (or what are business relations of Tuxera and Paragon), compared to
the FUSE-based driver, but ideally there's just one NTFS driver in linux
kernel.

The question I'd ask:

- what are users of current fs/ntfs driver going to lose, if the driver
  is going to be completely replaced by your driver

  If the answer is 'nothing' then, the straightfowrad plan is to just
  replace it. Otherwise, we'll have to figure that out.

2. split the patch

One patch of 27k lines of code is way too much to anybody to look at.

As an example, what worked for the recent EXFAT support, send each new
file as a patch.  This will pass the mailinglist size filters and keep
the patches logically separated, so eg. there are smaller patches
implementing interaction with VFS (on the inode or directory level) and
leaving out the other internal stuff of the filesystem.

I'm counting about 20 files, that's acceptable. The last patch, or maybe
a separate patch, adds the actual build and config-time support.

The situation is a bit more complicated as there's an existing driver
and I don't see a clear way how to do the transition from 2
implementations (one intermediate) to just one in the final state. We
have that already with the old VFAT and new EXFAT drivers, and it's
solved on the module level. Just one can be loaded (AFAIK). The same
could be done here but it puts some burden on users to know what driver
to load to get the expected set of features.

3. determine support status

You state intentions to work on the driver and there's a new entry in
MAINTAINERS file with 'Supported', so that's a good sign that it's not
just one-time dump of code. Fixing bugs or adding functionality is
expected.

4. development git tree

Once the filesystem is merged, you'd be getting mails from people
running eg. static checkers, build tests, sending cleanups or other
tree-wide changes.  The entry in MAINTAINER file does not point to any
git tree, so that's not clear to me what's the expected way to get the
fixes to Linus' tree or where are people supposed to look for 'is this
fixed already'.

There's maybe more I missed, but hopefully HTH.
d.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-15 19:06 ` David Sterba
@ 2020-08-16  0:42   ` Matthew Wilcox
  2020-08-20 10:48   ` Konstantin Komarov
  1 sibling, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2020-08-16  0:42 UTC (permalink / raw)
  To: dsterba, Konstantin Komarov, viro, linux-kernel, linux-fsdevel

On Sat, Aug 15, 2020 at 09:06:42PM +0200, David Sterba wrote:
> There's maybe more I missed, but hopefully HTH.

One thing you missed is adding support to fstests
git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git

If it passes that torture test, I think we can have confidence that
this is a really good implementation of a filesystem.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
                   ` (4 preceding siblings ...)
  2020-08-15 19:06 ` David Sterba
@ 2020-08-16  7:56 ` Pali Rohár
  5 siblings, 0 replies; 13+ messages in thread
From: Pali Rohár @ 2020-08-16  7:56 UTC (permalink / raw)
  To: Konstantin Komarov; +Cc: viro, linux-kernel, linux-fsdevel

On Friday 14 August 2020 12:29:01 Konstantin Komarov wrote:
> This patch adds NTFS Read-Write driver to fs/ntfs3.
> 
> Having decades of expertise in commercial file systems development and huge
> test coverage, we at Paragon Software GmbH want to make our contribution to
> the Open Source Community by providing implementation of NTFS Read-Write
> driver for the Linux Kernel.
> 
> This is fully functional NTFS Read-Write driver. Current version works with
> NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.
> 
> We plan to support this version after the codebase once merged, and add new
> features and fix bugs. For example, full journaling support over JBD will be
> added in later updates.
> 
> The patch is too big to handle it within an e-mail body, so it is available to download 
> on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch
> 
> Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

Hello Konstantin! I agree with David that patch is big and could be
split into smaller patches per file, like exfat driver was reviewed.

When you send a new version of ntfs driver, please add me to CC and I
will try to find some time to do code review. Thanks!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 13:40 ` David Sterba
@ 2020-08-20  9:26   ` Konstantin Komarov
  2020-08-20 10:59   ` Konstantin Komarov
  1 sibling, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-20  9:26 UTC (permalink / raw)
  To: dsterba; +Cc: viro, linux-kernel, linux-fsdevel

> From: David Sterba <dsterba@suse.cz>
> Sent: Friday, August 14, 2020 4:41 PM
> On Fri, Aug 14, 2020 at 12:29:01PM +0000, Konstantin Komarov wrote:
> > This patch adds NTFS Read-Write driver to fs/ntfs3.
> >
> > Having decades of expertise in commercial file systems development and huge
> > test coverage, we at Paragon Software GmbH want to make our contribution to
> > the Open Source Community by providing implementation of NTFS Read-Write
> > driver for the Linux Kernel.
> >
> > This is fully functional NTFS Read-Write driver. Current version works with
> > NTFS(including v3.1) normal/compressed/sparse files and supports journal replaying.
> >
> > We plan to support this version after the codebase once merged, and add new
> > features and fix bugs. For example, full journaling support over JBD will be
> > added in later updates.
> >
> > The patch is too big to handle it within an e-mail body, so it is available to download
> > on our server: https://dl.paragon-software.com/ntfs3/ntfs3.patch
> >
> > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
> 
> In case somebody wants to compile it, this fixup is needed to let 'make
> fs/ntfs3/' actually work, besides enabling it in the config.
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index 1c7b0e3f6daa..b0b4ad8affa0 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_SYSV_FS)		+= sysv/
>  obj-$(CONFIG_CIFS)		+= cifs/
>  obj-$(CONFIG_HPFS_FS)		+= hpfs/
>  obj-$(CONFIG_NTFS_FS)		+= ntfs/
> +obj-$(CONFIG_NTFS3_FS)		+= ntfs3/
>  obj-$(CONFIG_UFS_FS)		+= ufs/
>  obj-$(CONFIG_EFS_FS)		+= efs/
>  obj-$(CONFIG_JFFS2_FS)		+= jffs2/
> diff --git a/fs/ntfs3/Makefile b/fs/ntfs3/Makefile
> index 4d4fe198b8b8..d99dd1af43aa 100644
> --- a/fs/ntfs3/Makefile
> +++ b/fs/ntfs3/Makefile
> @@ -5,7 +5,7 @@
> 
>  obj-$(CONFIG_NTFS3_FS) += ntfs3.o
> 
> -ntfs3-objs := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
> +ntfs3-y := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
>  	    index.o attrlist.o record.o attrib.o run.o xattr.o\
>  	    upcase.o super.o file.o dir.o namei.o lznt.o\
>  	    fslog.o
> ---
Hi David, thanks! Indeed these fixups are needed to the patch (lost them during final polishing of the code before submitting). Will be fixed in v2.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 14:08 ` Aurélien Aptel
@ 2020-08-20 10:20   ` Konstantin Komarov
  0 siblings, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-20 10:20 UTC (permalink / raw)
  To: Aurélien Aptel, viro, linux-kernel, linux-fsdevel

From: Aurélien Aptel <aaptel@suse.com>
Sent: Friday, August 14, 2020 5:09 PM
> 
> Hi Konstantin,
> 
> That's cool :) As Nikolay said it needs a little change to the makefiles
> to even build.
> 
> Are you also going to publish your own mkfs.ntfs3 tool? I dont think the
> existing one would support 64k clusters.

Hi Aurélien. Thanks for your feedback. We plan to publish our mkfs.ntfs utility as the open source as well (and possibly fschk.ntfs - after mkfs). 

> 
> I would recommend to run checkpatch (I see already 87 warnings... some
> of it is noise):
> 
>   $ ./scripts/checkpatch.pl <patch>
> 
> And sparse (I dont see much):
> 
>   $ touch fs/ntfs3/*.[ch] && make C=1
> 
> You need a recent build of sparse to do that last one. You can pass your
> own sparse bin (make CHECK=~/prog/sparse/sparse C=1)
> 
> This will be a good first step.

The sparse utility is running against the code, as well as checkpatch.pl. Sprase output is clean now. Checkpatch's somehow important warnings will be fixed in v2 (except maybe typedefs). 

> 
> Have you tried to run the xfstests suite against it?
>
xfstests are being one of our standard test suites among others. Currently we have the 'generic/339' and 'generic/013' test cases failing, working on it now. Other tests either pass or being skipped (due to missing features e.g. reflink). 
 
> Cheers,
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 15:30 ` Aurélien Aptel
@ 2020-08-20 10:38   ` Konstantin Komarov
  0 siblings, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-20 10:38 UTC (permalink / raw)
  To: Aurélien Aptel, viro, linux-kernel, linux-fsdevel

From: Aurélien Aptel <aaptel@suse.com>
Sent: Friday, August 14, 2020 6:30 PM
> I've tried this using libntfs-3g mkfs.ntfs
> 
> # mkfs.ntfs /dev/vb1
> # mount -t ntfs3 /dev/vb1 /mnt
> 
> This already triggered UBSAN:
> Then I've tried to copy /etc into it:
> ...
> # cp -rp /etc /mnt
> 
> But this triggered a NULL ptr deref:
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000028

Thanks! This will be fixed in v2. To give some context: we use our mkfs utility in tests, coupled with a Windows-native one as a reference. P.S. Already have extended this approach with the current mkfs.ntfs utility as well.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-15 19:06 ` David Sterba
  2020-08-16  0:42   ` Matthew Wilcox
@ 2020-08-20 10:48   ` Konstantin Komarov
  1 sibling, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-20 10:48 UTC (permalink / raw)
  To: dsterba; +Cc: viro, linux-kernel, linux-fsdevel

From: David Sterba <dsterba@suse.cz>
Sent: Saturday, August 15, 2020 10:07 PM
> 
> 1. check existing support in kernel
> 
> There is fs/ntfs with read-only support from Tuxera, last change in the
> git tree is 3 years ago. The driver lacks read-write support so there's
> not 100% functionality duplication but there's still driver for the same
> filesystem.
> 
> I don't know what's the feature parity, how much the in-kernel driver is
> used (or what are business relations of Tuxera and Paragon), compared to
> the FUSE-based driver, but ideally there's just one NTFS driver in linux
> kernel.
> 
> The question I'd ask:
> 
> - what are users of current fs/ntfs driver going to lose, if the driver
>   is going to be completely replaced by your driver
> 
>   If the answer is 'nothing' then, the straightfowrad plan is to just
>   replace it. Otherwise, we'll have to figure that out.

Hi! Regarding the comparison with fs/ntfs driver - we of course did the analysis. There are two main points which make the difference: full read-write support (including compressed/sparse files) and journal replaying. The one thing which is missing in fs/ntfs3 in the inital patch is the appropriate processing of hybernated volumes (mounting them read-only to avoid corruptions). This, however, is considered as trivial change and will be added either in v2, or in v3. In general, we want to have the community's feedback on the topic, and if it's more suitable for the Linux Kernel not to have two implementations in Kernel, then the 'fs/ntfs' may be replaced.  

> 
> 2. split the patch
> 
> One patch of 27k lines of code is way too much to anybody to look at.

The patch will be splitted in v2 file-wise. Wasn't clear initially which way will be more convenient to review.

> 3. determine support status
> 
> You state intentions to work on the driver and there's a new entry in
> MAINTAINERS file with 'Supported', so that's a good sign that it's not
> just one-time dump of code. Fixing bugs or adding functionality is
> expected.
> 
> 4. development git tree
> 
> Once the filesystem is merged, you'd be getting mails from people
> running eg. static checkers, build tests, sending cleanups or other
> tree-wide changes.  The entry in MAINTAINER file does not point to any
> git tree, so that's not clear to me what's the expected way to get the
> fixes to Linus' tree or where are people supposed to look for 'is this
> fixed already'.

The external git repo for the code is currently being prepared, so it's the matter of time to have it. Will add the appropriate links to the MAINTERS once done.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software.
  2020-08-14 13:40 ` David Sterba
  2020-08-20  9:26   ` Konstantin Komarov
@ 2020-08-20 10:59   ` Konstantin Komarov
  1 sibling, 0 replies; 13+ messages in thread
From: Konstantin Komarov @ 2020-08-20 10:59 UTC (permalink / raw)
  To: dsterba; +Cc: viro, linux-kernel, linux-fsdevel

From: David Sterba <dsterba@suse.cz>
Sent: Friday, August 14, 2020 4:41 PM
> In case somebody wants to compile it, this fixup is needed to let 'make
> fs/ntfs3/' actually work, besides enabling it in the config.
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index 1c7b0e3f6daa..b0b4ad8affa0 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_SYSV_FS)		+= sysv/
>  obj-$(CONFIG_CIFS)		+= cifs/
>  obj-$(CONFIG_HPFS_FS)		+= hpfs/
>  obj-$(CONFIG_NTFS_FS)		+= ntfs/
> +obj-$(CONFIG_NTFS3_FS)		+= ntfs3/
>  obj-$(CONFIG_UFS_FS)		+= ufs/
>  obj-$(CONFIG_EFS_FS)		+= efs/
>  obj-$(CONFIG_JFFS2_FS)		+= jffs2/
> diff --git a/fs/ntfs3/Makefile b/fs/ntfs3/Makefile
> index 4d4fe198b8b8..d99dd1af43aa 100644
> --- a/fs/ntfs3/Makefile
> +++ b/fs/ntfs3/Makefile
> @@ -5,7 +5,7 @@
> 
>  obj-$(CONFIG_NTFS3_FS) += ntfs3.o
> 
> -ntfs3-objs := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
> +ntfs3-y := bitfunc.o bitmap.o inode.o fsntfs.o frecord.o \
>  	    index.o attrlist.o record.o attrib.o run.o xattr.o\
>  	    upcase.o super.o file.o dir.o namei.o lznt.o\
>  	    fslog.o

Thanks! Indeed these fixups are needed to the patch (lost them during final polishing of the code before submitting). Will be fixed in v2.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-08-20 13:55 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14 12:29 [PATCH] fs: NTFS read-write driver GPL implementation by Paragon Software Konstantin Komarov
2020-08-14 13:17 ` Nikolay Borisov
2020-08-14 13:40 ` David Sterba
2020-08-20  9:26   ` Konstantin Komarov
2020-08-20 10:59   ` Konstantin Komarov
2020-08-14 14:08 ` Aurélien Aptel
2020-08-20 10:20   ` Konstantin Komarov
2020-08-14 15:30 ` Aurélien Aptel
2020-08-20 10:38   ` Konstantin Komarov
2020-08-15 19:06 ` David Sterba
2020-08-16  0:42   ` Matthew Wilcox
2020-08-20 10:48   ` Konstantin Komarov
2020-08-16  7:56 ` Pali Rohár

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).