Sounds fine to me
On Sun, Sep 16, 2018 at 10:46 PM ronnie sahlberg
<ronniesahlberg@xxxxxxxxx> wrote:
>
> On Mon, Sep 17, 2018 at 3:47 AM, Aurélien Aptel <aaptel@xxxxxxxx> wrote:
> > Hi Ronnie,
> >
> > Ronnie Sahlberg <lsahlber@xxxxxxxxxx> writes:
> >> This allows userspace tools to query the raw info levels for cifs files
> >> and process the response in userspace.
> >> In particular this is useful for many of those data where there is no
> >> corresponding native data structure in linux.
> >> For example querying the security descriptor for a file and extract the
> >> SIDs.
> >
> > Does this overlap with the extended attributes thing we already have for
> > DOS attributes, SID, and ACL (setcifsacl/getcifsacl from cifs-utils)? I
> > have never used it so I cannot say for sure.
>
> Yeah, it is the same thing as that magic xattr.
>
> >
> >>
> >> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> >> ---
> >> fs/cifs/cifs_ioctl.h | 9 +++++++
> >> fs/cifs/cifsglob.h | 4 ++++
> >> fs/cifs/ioctl.c | 18 ++++++++++++++
> >> fs/cifs/smb2ops.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 98 insertions(+)
> >>
> >> diff --git a/fs/cifs/cifs_ioctl.h b/fs/cifs/cifs_ioctl.h
> >> index 57ff0756e30c..d8b7353260f6 100644
> >> --- a/fs/cifs/cifs_ioctl.h
> >> +++ b/fs/cifs/cifs_ioctl.h
> >> @@ -43,8 +43,17 @@ struct smb_snapshot_array {
> >> /* snapshots[]; */
> >> } __packed;
> >>
> >> +struct smb_query_info {
> >> + __u32 buffer_length;
> >> + __u32 info_type;
> >> + __u32 file_info_class;
> >> + __u32 additional_information;
> >> + /* char buffer[]; */
> >> +} __packed;
> >> +
> >> #define CIFS_IOCTL_MAGIC 0xCF
> >> #define CIFS_IOC_COPYCHUNK_FILE _IOW(CIFS_IOCTL_MAGIC, 3, int)
> >> #define CIFS_IOC_SET_INTEGRITY _IO(CIFS_IOCTL_MAGIC, 4)
> >> #define CIFS_IOC_GET_MNT_INFO _IOR(CIFS_IOCTL_MAGIC, 5, struct smb_mnt_fs_info)
> >> #define CIFS_ENUMERATE_SNAPSHOTS _IOR(CIFS_IOCTL_MAGIC, 6, struct smb_snapshot_array)
> >> +#define CIFS_QUERY_INFO _IOWR(CIFS_IOCTL_MAGIC, 7, struct smb_query_info)
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 9dcaed031843..bb9e5b90923d 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -465,6 +465,10 @@ struct smb_version_operations {
> >> enum securityEnum (*select_sectype)(struct TCP_Server_Info *,
> >> enum securityEnum);
> >> int (*next_header)(char *);
> >> + /* ioctl passthrough for query_info */
> >> + int (*ioctl_query_info)(const unsigned int xid,
> >> + struct cifsFileInfo *file,
> >> + unsigned long p);
> >
> > Shouldnt we put the void* __user thingy directly in the prototype or is
> > unsigned long the type already picked by the kernel ioctl code?
> >
> > Otherwise looks good. If Christoph Hellwig doesn't object to this new
> > ioctl we can use it in cifs-utils.
> >
> > I've made a little patch for cifs-utils (will send next to this) that
> > adds a cifsfileinfo utility. It's a slightly modified version of your
> > program (use defines for some constants, printf formats, getopt, -v
> > option, ...) and a man page. I wonder if it could/should be merged in
> > getcifsacl.
> >
> > The man page has a XXX part that needs to be replaced with the kernel
> > version if this makes it into a kernel release. Please speak up if you
> > think of a better name and feel free to complete the man page.
> >
> > Cheers,
> > --
> > Aurélien Aptel / SUSE Labs Samba Team
> > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
> > SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> > GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
Thanks,
Steve
