linux-kernel - Re: [PATCH v6 12/16] ntfs: add reparse and ea operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKYAXd9v_BzopZptrdeEONO9rdegT+XFVb0CLJHe16fDP9jNWQ@mail.gmail.com>
Date: Tue, 3 Feb 2026 23:17:24 +0900
From: Namjae Jeon <linkinjeon@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: viro@...iv.linux.org.uk, brauner@...nel.org, tytso@....edu, 
	willy@...radead.org, jack@...e.cz, djwong@...nel.org, josef@...icpanda.com, 
	sandeen@...deen.net, rgoldwyn@...e.com, xiang@...nel.org, dsterba@...e.com, 
	pali@...nel.org, ebiggers@...nel.org, neil@...wn.name, amir73il@...il.com, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	iamjoonsoo.kim@....com, cheol.lee@....com, jay.sim@....com, gunho.lee@....com
Subject: Re: [PATCH v6 12/16] ntfs: add reparse and ea operations

On Tue, Feb 3, 2026 at 3:38 PM Christoph Hellwig <hch@....de> wrote:
>
> Suggested commit message:
>
> Implement support for Extended Attributes and Reparse Points, enabling
> Posix ACL support and, and compatibility with Windows Subsystem for
> Linux (WSL) metadata.
Okay, I will use it.
>
> > +struct WSL_LINK_REPARSE_DATA {
> > +     __le32  type;
> > +     char    link[];
> > +};
> > +
> > +struct REPARSE_INDEX {                       /* index entry in $Extend/$Reparse */
>
> Why are these using all upper case names unlike the rest of the
> code?
Right, I will change it.
>
> > +     ok = ni && reparse_attr && (size >= sizeof(struct reparse_point)) &&
> > +             (reparse_attr->reparse_tag != IO_REPARSE_TAG_RESERVED_ZERO) &&
> > +             (((size_t)le16_to_cpu(reparse_attr->reparse_data_length) +
> > +               sizeof(struct reparse_point) +
> > +               ((reparse_attr->reparse_tag & IO_REPARSE_TAG_IS_MICROSOFT) ?
> > +                0 : sizeof(struct guid))) == size);
>
> A bunch of superflous braces.  But in general decomposing such complex
> operations into an inline helper using multiple if statements and
> adding comments improves the readability a lot.
Okay, I will update and add the comments.
>
> > +     if (ok) {
>
> ... and just return here for !ok and reduce the indentation for
> the rest of the function?
Right, I will change it like this.
>
> > +             switch (reparse_attr->reparse_tag) {
> > +             case IO_REPARSE_TAG_LX_SYMLINK:
> > +                     wsl_reparse_data = (const struct WSL_LINK_REPARSE_DATA *)
> > +                                             reparse_attr->reparse_data;
> > +                     if ((le16_to_cpu(reparse_attr->reparse_data_length) <=
> > +                          sizeof(wsl_reparse_data->type)) ||
> > +                         (wsl_reparse_data->type != cpu_to_le32(2)))
> > +                             ok = false;
> > +                     break;
> > +             case IO_REPARSE_TAG_AF_UNIX:
> > +             case IO_REPARSE_TAG_LX_FIFO:
> > +             case IO_REPARSE_TAG_LX_CHR:
> > +             case IO_REPARSE_TAG_LX_BLK:
> > +                     if (reparse_attr->reparse_data_length ||
> > +                         !(ni->flags & FILE_ATTRIBUTE_RECALL_ON_OPEN))
> > +                             ok = false;
> > +                     break;
>
> ... and then directly return from inside the switch as well?
Right, I will change it.
Thanks for the review!
>
>