| Message ID | 1367240239-19326-10-git-send-email-SteveD@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2013-04-29 at 08:57 -0400, Steve Dickson wrote: > From: David Quigley <dpquigl@davequigley.com> > > In order to mimic the way that NFSv4 ACLs are implemented we have created a > structure to be used to pass label data up and down the call chain. This patch > adds the new structure and new members to the required NFSv4 call structures. > > Signed-off-by: Matthew N. Dodd <Matthew.Dodd@sparta.com> > Signed-off-by: Miguel Rodel Felipe <Rodel_FM@dsi.a-star.edu.sg> > Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene@dsi.a-star.edu.sg> > Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG@dsi.a-star.edu.sg> > --- > fs/nfs/inode.c | 28 ++++++++++++++++++++++++++++ > include/linux/nfs4.h | 7 +++++++ > include/linux/nfs_fs.h | 18 ++++++++++++++++++ > include/linux/nfs_xdr.h | 21 +++++++++++++++++++++ > include/uapi/linux/nfs4.h | 2 +- > 5 files changed, 75 insertions(+), 1 deletion(-) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 1f94167..c88255c 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -257,6 +257,34 @@ nfs_init_locked(struct inode *inode, void *opaque) > return 0; > } > > +#ifdef CONFIG_NFS_V4_SECURITY_LABEL > +struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) > +{ > + struct nfs4_label *label = NULL; > + int minor_version = server->nfs_client->cl_minorversion; > + > + if (minor_version < 2) > + return label; > + > + if (!(server->caps & NFS_CAP_SECURITY_LABEL)) > + return label; > + > + label = kzalloc(sizeof(struct nfs4_label), flags); > + if (label == NULL) > + return ERR_PTR(-ENOMEM); > + > + label->label = kzalloc(NFS4_MAXLABELLEN, flags); > + if (label->label == NULL) { > + kfree(label); > + return ERR_PTR(-ENOMEM); > + } > + label->len = NFS4_MAXLABELLEN; > + > + return label; > +} > +EXPORT_SYMBOL_GPL(nfs4_label_alloc); > +#endif > + > /* > * This is our front-end to iget that looks up inodes by file handle > * instead of inode number. > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h > index 6f44cc1..4c61ef4 100644 > --- a/include/linux/nfs4.h > +++ b/include/linux/nfs4.h > @@ -32,6 +32,13 @@ struct nfs4_acl { > struct nfs4_ace aces[0]; > }; > > +struct nfs4_label { > + uint32_t lfs; > + uint32_t pi; > + u32 len; > + char *label; > +}; > + > typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier; > > struct nfs_stateid4 { > diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h > index 1cc2568..e0e1806 100644 > --- a/include/linux/nfs_fs.h > +++ b/include/linux/nfs_fs.h > @@ -489,6 +489,24 @@ extern int nfs_mountpoint_expiry_timeout; > extern void nfs_release_automount_timer(void); > > /* > + * linux/fs/nfs/nfs4proc.c > + */ > +#ifdef CONFIG_NFS_V4_SECURITY_LABEL > +extern struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags); > +static inline void nfs4_label_free(struct nfs4_label *label) > +{ > + if (label) { > + kfree(label->label); > + kfree(label); > + } > + return; > +} > +#else > +static inline struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) { return NULL; } > +static inline void nfs4_label_free(void *label) {} > +#endif > + > +/* > * linux/fs/nfs/unlink.c > */ > extern void nfs_complete_unlink(struct dentry *dentry, struct inode *); > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > index 9f2dba3..4d2fdf6 100644 > --- a/include/linux/nfs_xdr.h > +++ b/include/linux/nfs_xdr.h > @@ -351,6 +351,7 @@ struct nfs_openargs { > const u32 * bitmask; > const u32 * open_bitmap; > __u32 claim; > + const struct nfs4_label *label; > }; > > struct nfs_openres { > @@ -360,6 +361,7 @@ struct nfs_openres { > struct nfs4_change_info cinfo; > __u32 rflags; > struct nfs_fattr * f_attr; > + struct nfs4_label *f_label; > struct nfs_seqid * seqid; > const struct nfs_server *server; > fmode_t delegation_type; > @@ -404,6 +406,7 @@ struct nfs_closeres { > struct nfs4_sequence_res seq_res; > nfs4_stateid stateid; > struct nfs_fattr * fattr; > + struct nfs4_label *label; > struct nfs_seqid * seqid; > const struct nfs_server *server; > }; > @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { > struct nfs4_delegreturnres { > struct nfs4_sequence_res seq_res; > struct nfs_fattr * fattr; > + struct nfs4_label *label; > const struct nfs_server *server; > }; > > @@ -497,6 +501,7 @@ struct nfs_readargs { > struct nfs_readres { > struct nfs4_sequence_res seq_res; > struct nfs_fattr * fattr; > + struct nfs4_label *label; Why do we need to check labels on close, delegreturn, read, remove, rename, etc? Do any of those operations cause our cached labels to change?
On 01/05/13 14:59, Myklebust, Trond wrote: >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h >> > index 9f2dba3..4d2fdf6 100644 >> > --- a/include/linux/nfs_xdr.h >> > +++ b/include/linux/nfs_xdr.h >> > @@ -351,6 +351,7 @@ struct nfs_openargs { >> > const u32 * bitmask; >> > const u32 * open_bitmap; >> > __u32 claim; >> > + const struct nfs4_label *label; >> > }; >> > >> > struct nfs_openres { >> > @@ -360,6 +361,7 @@ struct nfs_openres { >> > struct nfs4_change_info cinfo; >> > __u32 rflags; >> > struct nfs_fattr * f_attr; >> > + struct nfs4_label *f_label; >> > struct nfs_seqid * seqid; >> > const struct nfs_server *server; >> > fmode_t delegation_type; >> > @@ -404,6 +406,7 @@ struct nfs_closeres { >> > struct nfs4_sequence_res seq_res; >> > nfs4_stateid stateid; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; >> > struct nfs_seqid * seqid; >> > const struct nfs_server *server; >> > }; >> > @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { >> > struct nfs4_delegreturnres { >> > struct nfs4_sequence_res seq_res; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; >> > const struct nfs_server *server; >> > }; >> > >> > @@ -497,6 +501,7 @@ struct nfs_readargs { >> > struct nfs_readres { >> > struct nfs4_sequence_res seq_res; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; > Why do we need to check labels on close, delegreturn, read, remove, > rename, etc? Do any of those operations cause our cached labels to > change? I was not around then this decision was made... Maybe some of the security people can address that.... What operation do you think they are needed for? steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-05-02 at 14:31 -0400, Steve Dickson wrote: > On 01/05/13 14:59, Myklebust, Trond wrote: > >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > >> > index 9f2dba3..4d2fdf6 100644 > >> > --- a/include/linux/nfs_xdr.h > >> > +++ b/include/linux/nfs_xdr.h > >> > @@ -351,6 +351,7 @@ struct nfs_openargs { > >> > const u32 * bitmask; > >> > const u32 * open_bitmap; > >> > __u32 claim; > >> > + const struct nfs4_label *label; > >> > }; > >> > > >> > struct nfs_openres { > >> > @@ -360,6 +361,7 @@ struct nfs_openres { > >> > struct nfs4_change_info cinfo; > >> > __u32 rflags; > >> > struct nfs_fattr * f_attr; > >> > + struct nfs4_label *f_label; > >> > struct nfs_seqid * seqid; > >> > const struct nfs_server *server; > >> > fmode_t delegation_type; > >> > @@ -404,6 +406,7 @@ struct nfs_closeres { > >> > struct nfs4_sequence_res seq_res; > >> > nfs4_stateid stateid; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > >> > struct nfs_seqid * seqid; > >> > const struct nfs_server *server; > >> > }; > >> > @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { > >> > struct nfs4_delegreturnres { > >> > struct nfs4_sequence_res seq_res; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > >> > const struct nfs_server *server; > >> > }; > >> > > >> > @@ -497,6 +501,7 @@ struct nfs_readargs { > >> > struct nfs_readres { > >> > struct nfs4_sequence_res seq_res; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > > Why do we need to check labels on close, delegreturn, read, remove, > > rename, etc? Do any of those operations cause our cached labels to > > change? > I was not around then this decision was made... Maybe some of the > security people can address that.... > > What operation do you think they are needed for? I would assume that they are only needed for the operations covered by nfs_atomic_open(), nfs_lookup(), nfs_revalidate_inode() and, of course, for nfs4_set_security_label(). All other cases are just opportunistic refreshes of the cache, which we have seen previously can have a nasty impact on NFSv4 performance.
On 01/05/13 14:59, Myklebust, Trond wrote: > On Mon, 2013-04-29 at 08:57 -0400, Steve Dickson wrote: >> From: David Quigley <dpquigl@davequigley.com> >> >> In order to mimic the way that NFSv4 ACLs are implemented we have created a >> structure to be used to pass label data up and down the call chain. This patch >> adds the new structure and new members to the required NFSv4 call structures. >> >> Signed-off-by: Matthew N. Dodd <Matthew.Dodd@sparta.com> >> Signed-off-by: Miguel Rodel Felipe <Rodel_FM@dsi.a-star.edu.sg> >> Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene@dsi.a-star.edu.sg> >> Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG@dsi.a-star.edu.sg> >> --- >> fs/nfs/inode.c | 28 ++++++++++++++++++++++++++++ >> include/linux/nfs4.h | 7 +++++++ >> include/linux/nfs_fs.h | 18 ++++++++++++++++++ >> include/linux/nfs_xdr.h | 21 +++++++++++++++++++++ >> include/uapi/linux/nfs4.h | 2 +- >> 5 files changed, 75 insertions(+), 1 deletion(-) >> >> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >> index 1f94167..c88255c 100644 >> --- a/fs/nfs/inode.c >> +++ b/fs/nfs/inode.c >> @@ -257,6 +257,34 @@ nfs_init_locked(struct inode *inode, void *opaque) >> return 0; >> } >> >> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL >> +struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) >> +{ >> + struct nfs4_label *label = NULL; >> + int minor_version = server->nfs_client->cl_minorversion; >> + >> + if (minor_version < 2) >> + return label; >> + >> + if (!(server->caps & NFS_CAP_SECURITY_LABEL)) >> + return label; >> + >> + label = kzalloc(sizeof(struct nfs4_label), flags); >> + if (label == NULL) >> + return ERR_PTR(-ENOMEM); >> + >> + label->label = kzalloc(NFS4_MAXLABELLEN, flags); >> + if (label->label == NULL) { >> + kfree(label); >> + return ERR_PTR(-ENOMEM); >> + } >> + label->len = NFS4_MAXLABELLEN; >> + >> + return label; >> +} >> +EXPORT_SYMBOL_GPL(nfs4_label_alloc); >> +#endif >> + >> /* >> * This is our front-end to iget that looks up inodes by file handle >> * instead of inode number. >> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >> index 6f44cc1..4c61ef4 100644 >> --- a/include/linux/nfs4.h >> +++ b/include/linux/nfs4.h >> @@ -32,6 +32,13 @@ struct nfs4_acl { >> struct nfs4_ace aces[0]; >> }; >> >> +struct nfs4_label { >> + uint32_t lfs; >> + uint32_t pi; >> + u32 len; >> + char *label; >> +}; >> + >> typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier; >> >> struct nfs_stateid4 { >> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h >> index 1cc2568..e0e1806 100644 >> --- a/include/linux/nfs_fs.h >> +++ b/include/linux/nfs_fs.h >> @@ -489,6 +489,24 @@ extern int nfs_mountpoint_expiry_timeout; >> extern void nfs_release_automount_timer(void); >> >> /* >> + * linux/fs/nfs/nfs4proc.c >> + */ >> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL >> +extern struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags); >> +static inline void nfs4_label_free(struct nfs4_label *label) >> +{ >> + if (label) { >> + kfree(label->label); >> + kfree(label); >> + } >> + return; >> +} >> +#else >> +static inline struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) { return NULL; } >> +static inline void nfs4_label_free(void *label) {} >> +#endif >> + >> +/* >> * linux/fs/nfs/unlink.c >> */ >> extern void nfs_complete_unlink(struct dentry *dentry, struct inode *); >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h >> index 9f2dba3..4d2fdf6 100644 >> --- a/include/linux/nfs_xdr.h >> +++ b/include/linux/nfs_xdr.h >> @@ -351,6 +351,7 @@ struct nfs_openargs { >> const u32 * bitmask; >> const u32 * open_bitmap; >> __u32 claim; >> + const struct nfs4_label *label; >> }; >> >> struct nfs_openres { >> @@ -360,6 +361,7 @@ struct nfs_openres { >> struct nfs4_change_info cinfo; >> __u32 rflags; >> struct nfs_fattr * f_attr; >> + struct nfs4_label *f_label; >> struct nfs_seqid * seqid; >> const struct nfs_server *server; >> fmode_t delegation_type; >> @@ -404,6 +406,7 @@ struct nfs_closeres { >> struct nfs4_sequence_res seq_res; >> nfs4_stateid stateid; >> struct nfs_fattr * fattr; >> + struct nfs4_label *label; >> struct nfs_seqid * seqid; >> const struct nfs_server *server; >> }; >> @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { >> struct nfs4_delegreturnres { >> struct nfs4_sequence_res seq_res; >> struct nfs_fattr * fattr; >> + struct nfs4_label *label; >> const struct nfs_server *server; >> }; >> >> @@ -497,6 +501,7 @@ struct nfs_readargs { >> struct nfs_readres { >> struct nfs4_sequence_res seq_res; >> struct nfs_fattr * fattr; >> + struct nfs4_label *label; > > Why do we need to check labels on close, delegreturn, read, remove, > rename, etc? Do any of those operations cause our cached labels to > change? > Being I've got complete silence from the security community for over a week now (publicly and privately), I'm going to assume the answer to your questions is No and No... and I'll start working on ripping the code out today... steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/8/2013 6:43 AM, Steve Dickson wrote: > > On 01/05/13 14:59, Myklebust, Trond wrote: >> On Mon, 2013-04-29 at 08:57 -0400, Steve Dickson wrote: >>> From: David Quigley <dpquigl@davequigley.com> >>> >>> In order to mimic the way that NFSv4 ACLs are implemented we have created a >>> structure to be used to pass label data up and down the call chain. This patch >>> adds the new structure and new members to the required NFSv4 call structures. >>> >>> Signed-off-by: Matthew N. Dodd <Matthew.Dodd@sparta.com> >>> Signed-off-by: Miguel Rodel Felipe <Rodel_FM@dsi.a-star.edu.sg> >>> Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene@dsi.a-star.edu.sg> >>> Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG@dsi.a-star.edu.sg> >>> --- >>> fs/nfs/inode.c | 28 ++++++++++++++++++++++++++++ >>> include/linux/nfs4.h | 7 +++++++ >>> include/linux/nfs_fs.h | 18 ++++++++++++++++++ >>> include/linux/nfs_xdr.h | 21 +++++++++++++++++++++ >>> include/uapi/linux/nfs4.h | 2 +- >>> 5 files changed, 75 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c >>> index 1f94167..c88255c 100644 >>> --- a/fs/nfs/inode.c >>> +++ b/fs/nfs/inode.c >>> @@ -257,6 +257,34 @@ nfs_init_locked(struct inode *inode, void *opaque) >>> return 0; >>> } >>> >>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL >>> +struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) >>> +{ >>> + struct nfs4_label *label = NULL; >>> + int minor_version = server->nfs_client->cl_minorversion; >>> + >>> + if (minor_version < 2) >>> + return label; >>> + >>> + if (!(server->caps & NFS_CAP_SECURITY_LABEL)) >>> + return label; >>> + >>> + label = kzalloc(sizeof(struct nfs4_label), flags); >>> + if (label == NULL) >>> + return ERR_PTR(-ENOMEM); >>> + >>> + label->label = kzalloc(NFS4_MAXLABELLEN, flags); >>> + if (label->label == NULL) { >>> + kfree(label); >>> + return ERR_PTR(-ENOMEM); >>> + } >>> + label->len = NFS4_MAXLABELLEN; >>> + >>> + return label; >>> +} >>> +EXPORT_SYMBOL_GPL(nfs4_label_alloc); >>> +#endif >>> + >>> /* >>> * This is our front-end to iget that looks up inodes by file handle >>> * instead of inode number. >>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h >>> index 6f44cc1..4c61ef4 100644 >>> --- a/include/linux/nfs4.h >>> +++ b/include/linux/nfs4.h >>> @@ -32,6 +32,13 @@ struct nfs4_acl { >>> struct nfs4_ace aces[0]; >>> }; >>> >>> +struct nfs4_label { >>> + uint32_t lfs; >>> + uint32_t pi; >>> + u32 len; >>> + char *label; >>> +}; >>> + >>> typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier; >>> >>> struct nfs_stateid4 { >>> diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h >>> index 1cc2568..e0e1806 100644 >>> --- a/include/linux/nfs_fs.h >>> +++ b/include/linux/nfs_fs.h >>> @@ -489,6 +489,24 @@ extern int nfs_mountpoint_expiry_timeout; >>> extern void nfs_release_automount_timer(void); >>> >>> /* >>> + * linux/fs/nfs/nfs4proc.c >>> + */ >>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL >>> +extern struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags); >>> +static inline void nfs4_label_free(struct nfs4_label *label) >>> +{ >>> + if (label) { >>> + kfree(label->label); >>> + kfree(label); >>> + } >>> + return; >>> +} >>> +#else >>> +static inline struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) { return NULL; } >>> +static inline void nfs4_label_free(void *label) {} >>> +#endif >>> + >>> +/* >>> * linux/fs/nfs/unlink.c >>> */ >>> extern void nfs_complete_unlink(struct dentry *dentry, struct inode *); >>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h >>> index 9f2dba3..4d2fdf6 100644 >>> --- a/include/linux/nfs_xdr.h >>> +++ b/include/linux/nfs_xdr.h >>> @@ -351,6 +351,7 @@ struct nfs_openargs { >>> const u32 * bitmask; >>> const u32 * open_bitmap; >>> __u32 claim; >>> + const struct nfs4_label *label; >>> }; >>> >>> struct nfs_openres { >>> @@ -360,6 +361,7 @@ struct nfs_openres { >>> struct nfs4_change_info cinfo; >>> __u32 rflags; >>> struct nfs_fattr * f_attr; >>> + struct nfs4_label *f_label; >>> struct nfs_seqid * seqid; >>> const struct nfs_server *server; >>> fmode_t delegation_type; >>> @@ -404,6 +406,7 @@ struct nfs_closeres { >>> struct nfs4_sequence_res seq_res; >>> nfs4_stateid stateid; >>> struct nfs_fattr * fattr; >>> + struct nfs4_label *label; >>> struct nfs_seqid * seqid; >>> const struct nfs_server *server; >>> }; >>> @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { >>> struct nfs4_delegreturnres { >>> struct nfs4_sequence_res seq_res; >>> struct nfs_fattr * fattr; >>> + struct nfs4_label *label; >>> const struct nfs_server *server; >>> }; >>> >>> @@ -497,6 +501,7 @@ struct nfs_readargs { >>> struct nfs_readres { >>> struct nfs4_sequence_res seq_res; >>> struct nfs_fattr * fattr; >>> + struct nfs4_label *label; >> Why do we need to check labels on close, delegreturn, read, remove, >> rename, etc? Do any of those operations cause our cached labels to >> change? >> > Being I've got complete silence from the security community for > over a week now (publicly and privately), I'm going to assume the answer > to your questions is No and No... and I'll start working on ripping the > code out today... As I've stated earlier, I have not been able to get the labeled NFS working with Smack and until I can (or someone else can) I can't say much about the existing or proposed behavior. My understanding is that there are SELinux people directly involved in the labeled NFS effort, so you should be getting comment directly if they care. > > steved. > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/05/13 14:59, Myklebust, Trond wrote: >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h >> > index 9f2dba3..4d2fdf6 100644 >> > --- a/include/linux/nfs_xdr.h >> > +++ b/include/linux/nfs_xdr.h >> > @@ -351,6 +351,7 @@ struct nfs_openargs { >> > const u32 * bitmask; >> > const u32 * open_bitmap; >> > __u32 claim; >> > + const struct nfs4_label *label; >> > }; >> > >> > struct nfs_openres { >> > @@ -360,6 +361,7 @@ struct nfs_openres { >> > struct nfs4_change_info cinfo; >> > __u32 rflags; >> > struct nfs_fattr * f_attr; >> > + struct nfs4_label *f_label; >> > struct nfs_seqid * seqid; >> > const struct nfs_server *server; >> > fmode_t delegation_type; >> > @@ -404,6 +406,7 @@ struct nfs_closeres { >> > struct nfs4_sequence_res seq_res; >> > nfs4_stateid stateid; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; >> > struct nfs_seqid * seqid; >> > const struct nfs_server *server; >> > }; >> > @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { >> > struct nfs4_delegreturnres { >> > struct nfs4_sequence_res seq_res; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; >> > const struct nfs_server *server; >> > }; >> > >> > @@ -497,6 +501,7 @@ struct nfs_readargs { >> > struct nfs_readres { >> > struct nfs4_sequence_res seq_res; >> > struct nfs_fattr * fattr; >> > + struct nfs4_label *label; > Why do we need to check labels on close, delegreturn, read, remove, > rename, etc? All of these have GETATTR in the compound they send out. So I'm assuming to keep consistence, the label bit needs to be set on all *most* of the GEATTRs. Ones that actually nfs_refresh_inode() the inode. > Do any of those operations cause our cached labels to change? Not the label but the inode. The bit is set in ACCESS, LOOKUP, LINK, OPEN, CREAT, SETATTR which all clearly update the inode. So I guess my question is if the bit is not set in any of these ops how do we know if the label has changed? Should label changes be synchronized with inode updates? steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-05-08 at 13:32 -0400, Steve Dickson wrote: > On 01/05/13 14:59, Myklebust, Trond wrote: > >> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h > >> > index 9f2dba3..4d2fdf6 100644 > >> > --- a/include/linux/nfs_xdr.h > >> > +++ b/include/linux/nfs_xdr.h > >> > @@ -351,6 +351,7 @@ struct nfs_openargs { > >> > const u32 * bitmask; > >> > const u32 * open_bitmap; > >> > __u32 claim; > >> > + const struct nfs4_label *label; > >> > }; > >> > > >> > struct nfs_openres { > >> > @@ -360,6 +361,7 @@ struct nfs_openres { > >> > struct nfs4_change_info cinfo; > >> > __u32 rflags; > >> > struct nfs_fattr * f_attr; > >> > + struct nfs4_label *f_label; > >> > struct nfs_seqid * seqid; > >> > const struct nfs_server *server; > >> > fmode_t delegation_type; > >> > @@ -404,6 +406,7 @@ struct nfs_closeres { > >> > struct nfs4_sequence_res seq_res; > >> > nfs4_stateid stateid; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > >> > struct nfs_seqid * seqid; > >> > const struct nfs_server *server; > >> > }; > >> > @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { > >> > struct nfs4_delegreturnres { > >> > struct nfs4_sequence_res seq_res; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > >> > const struct nfs_server *server; > >> > }; > >> > > >> > @@ -497,6 +501,7 @@ struct nfs_readargs { > >> > struct nfs_readres { > >> > struct nfs4_sequence_res seq_res; > >> > struct nfs_fattr * fattr; > >> > + struct nfs4_label *label; > > Why do we need to check labels on close, delegreturn, read, remove, > > rename, etc? > All of these have GETATTR in the compound they send out. So I'm > assuming to keep consistence, the label bit needs to be set > on all *most* of the GEATTRs. Ones that actually nfs_refresh_inode() > the inode. We need to distinguish between cache consistency GETATTRs, and metadata refresh GETATTRs. Cache consistency GETATTRs are basically all about making sure that we're keeping the change_attribute (and size) up to date so that we know whether or not the data and metadata caches are correct. Metadata refresh GETATTRs are sent if we know or suspect that our cached metadata is incorrect, and we need to _use_ some of that cached data (in a stat() syscall, or as part of a path lookup or an open()). > > Do any of those operations cause our cached labels to change? > Not the label but the inode. The bit is set in ACCESS, LOOKUP, > LINK, OPEN, CREAT, SETATTR which all clearly update the inode. > > So I guess my question is if the bit is not set in any of these > ops how do we know if the label has changed? Should label changes > be synchronized with inode updates? I know that Bruce doesn't like FATTR4_CHANGE_SEC_LABEL, but that is the only option I can see for implementing a cache consistency model for labels. Without it, the choices are: 1) always fetch the label as part of every COMPOUND. 2) assume the label never changes on the server. The main use cases that have been presented for Labeled NFS on Linux would tend to push me towards door number 2, Monty please...
On Wed, May 08, 2013 at 06:06:24PM +0000, Myklebust, Trond wrote: > On Wed, 2013-05-08 at 13:32 -0400, Steve Dickson wrote: > > Not the label but the inode. The bit is set in ACCESS, LOOKUP, > > LINK, OPEN, CREAT, SETATTR which all clearly update the inode. > > > > So I guess my question is if the bit is not set in any of these > > ops how do we know if the label has changed? Should label changes > > be synchronized with inode updates? > > I know that Bruce doesn't like FATTR4_CHANGE_SEC_LABEL, Not true at all, sorry to give that impression. It doesn't look like a lot of work to implement (how much work would it be to use on the client side?), and I'd be perfectly happy if someone would do so. It's just that noone's volunteered, and my todo list is out of control as it is. > but that is the > only option I can see for implementing a cache consistency model for > labels. Without it, the choices are: > > 1) always fetch the label as part of every COMPOUND. > 2) assume the label never changes on the server. > > The main use cases that have been presented for Labeled NFS on Linux > would tend to push me towards door number 2, Monty please... And that's fine with me too. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/05/13 14:56, J. Bruce Fields wrote: > On Wed, May 08, 2013 at 06:06:24PM +0000, Myklebust, Trond wrote: >> On Wed, 2013-05-08 at 13:32 -0400, Steve Dickson wrote: >>> Not the label but the inode. The bit is set in ACCESS, LOOKUP, >>> LINK, OPEN, CREAT, SETATTR which all clearly update the inode. >>> >>> So I guess my question is if the bit is not set in any of these >>> ops how do we know if the label has changed? Should label changes >>> be synchronized with inode updates? >> >> I know that Bruce doesn't like FATTR4_CHANGE_SEC_LABEL, > > Not true at all, sorry to give that impression. > > It doesn't look like a lot of work to implement (how much work would it > be to use on the client side?), and I'd be perfectly happy if someone > would do so. > > It's just that noone's volunteered, and my todo list is out of control > as it is. > >> but that is the >> only option I can see for implementing a cache consistency model for >> labels. Without it, the choices are: >> >> 1) always fetch the label as part of every COMPOUND. >> 2) assume the label never changes on the server. >> >> The main use cases that have been presented for Labeled NFS on Linux >> would tend to push me towards door number 2, Monty please... > > And that's fine with me too. This being the case... Are you saying setting the label only needs to be set during the nfs_fhget(), _nfs4_do_open() and nfs4_set_security_label(). Which means the setting of the label will be removed from nfs_refresh_inode() and all its callers.... correct? steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 1f94167..c88255c 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -257,6 +257,34 @@ nfs_init_locked(struct inode *inode, void *opaque) return 0; } +#ifdef CONFIG_NFS_V4_SECURITY_LABEL +struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) +{ + struct nfs4_label *label = NULL; + int minor_version = server->nfs_client->cl_minorversion; + + if (minor_version < 2) + return label; + + if (!(server->caps & NFS_CAP_SECURITY_LABEL)) + return label; + + label = kzalloc(sizeof(struct nfs4_label), flags); + if (label == NULL) + return ERR_PTR(-ENOMEM); + + label->label = kzalloc(NFS4_MAXLABELLEN, flags); + if (label->label == NULL) { + kfree(label); + return ERR_PTR(-ENOMEM); + } + label->len = NFS4_MAXLABELLEN; + + return label; +} +EXPORT_SYMBOL_GPL(nfs4_label_alloc); +#endif + /* * This is our front-end to iget that looks up inodes by file handle * instead of inode number. diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h index 6f44cc1..4c61ef4 100644 --- a/include/linux/nfs4.h +++ b/include/linux/nfs4.h @@ -32,6 +32,13 @@ struct nfs4_acl { struct nfs4_ace aces[0]; }; +struct nfs4_label { + uint32_t lfs; + uint32_t pi; + u32 len; + char *label; +}; + typedef struct { char data[NFS4_VERIFIER_SIZE]; } nfs4_verifier; struct nfs_stateid4 { diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h index 1cc2568..e0e1806 100644 --- a/include/linux/nfs_fs.h +++ b/include/linux/nfs_fs.h @@ -489,6 +489,24 @@ extern int nfs_mountpoint_expiry_timeout; extern void nfs_release_automount_timer(void); /* + * linux/fs/nfs/nfs4proc.c + */ +#ifdef CONFIG_NFS_V4_SECURITY_LABEL +extern struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags); +static inline void nfs4_label_free(struct nfs4_label *label) +{ + if (label) { + kfree(label->label); + kfree(label); + } + return; +} +#else +static inline struct nfs4_label *nfs4_label_alloc(struct nfs_server *server, gfp_t flags) { return NULL; } +static inline void nfs4_label_free(void *label) {} +#endif + +/* * linux/fs/nfs/unlink.c */ extern void nfs_complete_unlink(struct dentry *dentry, struct inode *); diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 9f2dba3..4d2fdf6 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -351,6 +351,7 @@ struct nfs_openargs { const u32 * bitmask; const u32 * open_bitmap; __u32 claim; + const struct nfs4_label *label; }; struct nfs_openres { @@ -360,6 +361,7 @@ struct nfs_openres { struct nfs4_change_info cinfo; __u32 rflags; struct nfs_fattr * f_attr; + struct nfs4_label *f_label; struct nfs_seqid * seqid; const struct nfs_server *server; fmode_t delegation_type; @@ -404,6 +406,7 @@ struct nfs_closeres { struct nfs4_sequence_res seq_res; nfs4_stateid stateid; struct nfs_fattr * fattr; + struct nfs4_label *label; struct nfs_seqid * seqid; const struct nfs_server *server; }; @@ -477,6 +480,7 @@ struct nfs4_delegreturnargs { struct nfs4_delegreturnres { struct nfs4_sequence_res seq_res; struct nfs_fattr * fattr; + struct nfs4_label *label; const struct nfs_server *server; }; @@ -497,6 +501,7 @@ struct nfs_readargs { struct nfs_readres { struct nfs4_sequence_res seq_res; struct nfs_fattr * fattr; + struct nfs4_label *label; __u32 count; int eof; }; @@ -565,6 +570,7 @@ struct nfs_removeres { struct nfs4_sequence_res seq_res; const struct nfs_server *server; struct nfs_fattr *dir_attr; + struct nfs4_label *dir_label; struct nfs4_change_info cinfo; }; @@ -577,6 +583,8 @@ struct nfs_renameargs { const struct nfs_fh *new_dir; const struct qstr *old_name; const struct qstr *new_name; + const struct nfs4_label *old_label; + const struct nfs4_label *new_label; }; struct nfs_renameres { @@ -584,8 +592,10 @@ struct nfs_renameres { const struct nfs_server *server; struct nfs4_change_info old_cinfo; struct nfs_fattr *old_fattr; + struct nfs4_label *old_label; struct nfs4_change_info new_cinfo; struct nfs_fattr *new_fattr; + struct nfs4_label *new_label; }; /* @@ -600,6 +610,7 @@ struct nfs_entry { int eof; struct nfs_fh * fh; struct nfs_fattr * fattr; + struct nfs4_label *label; unsigned char d_type; struct nfs_server * server; }; @@ -632,6 +643,7 @@ struct nfs_setattrargs { struct iattr * iap; const struct nfs_server * server; /* Needed for name mapping */ const u32 * bitmask; + const struct nfs4_label *label; }; struct nfs_setaclargs { @@ -667,6 +679,7 @@ struct nfs_getaclres { struct nfs_setattrres { struct nfs4_sequence_res seq_res; struct nfs_fattr * fattr; + struct nfs4_label *label; const struct nfs_server * server; }; @@ -712,6 +725,7 @@ struct nfs3_setaclargs { struct nfs_diropok { struct nfs_fh * fh; struct nfs_fattr * fattr; + struct nfs4_label *label; }; struct nfs_readlinkargs { @@ -842,6 +856,7 @@ struct nfs4_accessres { struct nfs4_sequence_res seq_res; const struct nfs_server * server; struct nfs_fattr * fattr; + struct nfs4_label *label; u32 supported; u32 access; }; @@ -864,6 +879,7 @@ struct nfs4_create_arg { const struct iattr * attrs; const struct nfs_fh * dir_fh; const u32 * bitmask; + const struct nfs4_label *label; }; struct nfs4_create_res { @@ -871,6 +887,7 @@ struct nfs4_create_res { const struct nfs_server * server; struct nfs_fh * fh; struct nfs_fattr * fattr; + struct nfs4_label *label; struct nfs4_change_info dir_cinfo; }; @@ -895,6 +912,7 @@ struct nfs4_getattr_res { struct nfs4_sequence_res seq_res; const struct nfs_server * server; struct nfs_fattr * fattr; + struct nfs4_label *label; }; struct nfs4_link_arg { @@ -909,8 +927,10 @@ struct nfs4_link_res { struct nfs4_sequence_res seq_res; const struct nfs_server * server; struct nfs_fattr * fattr; + struct nfs4_label *label; struct nfs4_change_info cinfo; struct nfs_fattr * dir_attr; + struct nfs4_label *dir_label; }; @@ -926,6 +946,7 @@ struct nfs4_lookup_res { const struct nfs_server * server; struct nfs_fattr * fattr; struct nfs_fh * fh; + struct nfs4_label *label; }; struct nfs4_lookup_root_arg { diff --git a/include/uapi/linux/nfs4.h b/include/uapi/linux/nfs4.h index 788128e..78d25b5 100644 --- a/include/uapi/linux/nfs4.h +++ b/include/uapi/linux/nfs4.h @@ -25,7 +25,7 @@ #define NFS4_MAXNAMLEN NAME_MAX #define NFS4_OPAQUE_LIMIT 1024 #define NFS4_MAX_SESSIONID_LEN 16 - +#define NFS4_MAXLABELLEN 2048 #define NFS4_ACCESS_READ 0x0001 #define NFS4_ACCESS_LOOKUP 0x0002 #define NFS4_ACCESS_MODIFY 0x0004