[PATCH] security/security: Convert to timespec64 type for "settime" pointer of struct security_operations
This patch changes the "settime" pointer's argument with timespec64 type in security_operations structure, and also introduces the security_settime64() function with timespec64 type, meanwhile moves the security_settime() defined in security/security.c file to include/linux/security.h file as a static function.
Also introduces a default security_settime64() function with timespec64 type when it doesn't define the CONFIG_SECURITY macro. And change the cap_settime() function's argument with timespec64 type, which is only used by security_settime64() function.
Signed-off-by: Baolin Wang baolin.wang@linaro.org --- include/linux/security.h | 25 ++++++++++++++++++++----- security/commoncap.c | 2 +- security/security.c | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h index a1b7dbd..69ad545 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -75,7 +75,7 @@ struct timezone; */ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, int cap, int audit); -extern int cap_settime(const struct timespec *ts, const struct timezone *tz); +extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz); extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode); extern int cap_ptrace_traceme(struct task_struct *parent); extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); @@ -1353,7 +1353,8 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts) * Return 0 if permission is granted. * @settime: * Check permission to change the system time. - * struct timespec and timezone are defined in include/linux/time.h + * struct timespec64 is defined in include/linux/time64.h and timezone is + * defined in include/linux/time.h * @ts contains new time * @tz contains new timezone * Return 0 if permission is granted. @@ -1483,7 +1484,7 @@ struct security_operations { int (*quotactl) (int cmds, int type, int id, struct super_block *sb); int (*quota_on) (struct dentry *dentry); int (*syslog) (int type); - int (*settime) (const struct timespec *ts, const struct timezone *tz); + int (*settime) (const struct timespec64 *ts, const struct timezone *tz); int (*vm_enough_memory) (struct mm_struct *mm, long pages);
int (*bprm_set_creds) (struct linux_binprm *bprm); @@ -1790,7 +1791,13 @@ int security_capable_noaudit(const struct cred *cred, struct user_namespace *ns, int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); -int security_settime(const struct timespec *ts, const struct timezone *tz); +int security_settime64(const struct timespec64 *ts, const struct timezone *tz); +static int security_settime(const struct timespec *ts, const struct timezone *tz) +{ + struct timespec64 ts64 = timespec_to_timespec64(*ts); + + return security_settime64(&ts64, tz); +} int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); @@ -2040,10 +2047,18 @@ static inline int security_syslog(int type) return 0; }
+static inline int security_settime64(const struct timespec64 *ts, + const struct timezone *tz) +{ + return cap_settime(ts, tz); +} + static inline int security_settime(const struct timespec *ts, const struct timezone *tz) { - return cap_settime(ts, tz); + struct timsepc64 ts64 = timespec_to_timespec64(*ts); + + return security_settime64(&ts64, tz); }
static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) diff --git a/security/commoncap.c b/security/commoncap.c index f66713b..bdb8ec0 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -116,7 +116,7 @@ int cap_capable(const struct cred *cred, struct user_namespace *targ_ns, * Determine whether the current process may set the system clock and timezone * information, returning 0 if permission granted, -ve if denied. */ -int cap_settime(const struct timespec *ts, const struct timezone *tz) +int cap_settime(const struct timespec64 *ts, const struct timezone *tz) { if (!capable(CAP_SYS_TIME)) return -EPERM; diff --git a/security/security.c b/security/security.c index e81d5bb..0be5032 100644 --- a/security/security.c +++ b/security/security.c @@ -224,7 +224,7 @@ int security_syslog(int type) return security_ops->syslog(type); }
-int security_settime(const struct timespec *ts, const struct timezone *tz) +int security_settime64(const struct timespec64 *ts, const struct timezone *tz) { return security_ops->settime(ts, tz); }
On 14 May 2015 at 15:03, Baolin Wang baolin.wang@linaro.org wrote:
This patch changes the "settime" pointer's argument with timespec64 type in security_operations structure, and also introduces the security_settime64() function with timespec64 type, meanwhile moves the security_settime() defined in security/security.c file to include/linux/security.h file as a static function.
Also introduces a default security_settime64() function with timespec64 type when it doesn't define the CONFIG_SECURITY macro. And change the cap_settime() function's argument with timespec64 type, which is only used by security_settime64() function.
Signed-off-by: Baolin Wang baolin.wang@linaro.org
include/linux/security.h | 25 ++++++++++++++++++++----- security/commoncap.c | 2 +- security/security.c | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h index a1b7dbd..69ad545 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -75,7 +75,7 @@ struct timezone; */ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, int cap, int audit); -extern int cap_settime(const struct timespec *ts, const struct timezone *tz); +extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz); extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode); extern int cap_ptrace_traceme(struct task_struct *parent); extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted); @@ -1353,7 +1353,8 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
Return 0 if permission is granted.- @settime:
Check permission to change the system time.
struct timespec and timezone are defined in include/linux/time.h
struct timespec64 is defined in include/linux/time64.h andtimezone is
defined in include/linux/time.h@ts contains new time@tz contains new timezoneReturn 0 if permission is granted.@@ -1483,7 +1484,7 @@ struct security_operations { int (*quotactl) (int cmds, int type, int id, struct super_block *sb); int (*quota_on) (struct dentry *dentry); int (*syslog) (int type);
int (*settime) (const struct timespec *ts, const struct timezone*tz);
int (*settime) (const struct timespec64 *ts, const struct timezone*tz); int (*vm_enough_memory) (struct mm_struct *mm, long pages);
int (*bprm_set_creds) (struct linux_binprm *bprm);@@ -1790,7 +1791,13 @@ int security_capable_noaudit(const struct cred *cred, struct user_namespace *ns, int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); -int security_settime(const struct timespec *ts, const struct timezone *tz); +int security_settime64(const struct timespec64 *ts, const struct timezone *tz); +static int security_settime(const struct timespec *ts, const struct timezone *tz) +{
struct timespec64 ts64 = timespec_to_timespec64(*ts);return security_settime64(&ts64, tz);+} int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); @@ -2040,10 +2047,18 @@ static inline int security_syslog(int type) return 0; }
+static inline int security_settime64(const struct timespec64 *ts,
const struct timezone *tz)+{
return cap_settime(ts, tz);+}
static inline int security_settime(const struct timespec *ts, const struct timezone *tz) {
return cap_settime(ts, tz);
struct timsepc64 ts64 = timespec_to_timespec64(*ts);return security_settime64(&ts64, tz);}
static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) diff --git a/security/commoncap.c b/security/commoncap.c index f66713b..bdb8ec0 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -116,7 +116,7 @@ int cap_capable(const struct cred *cred, struct user_namespace *targ_ns,
- Determine whether the current process may set the system clock and
timezone
- information, returning 0 if permission granted, -ve if denied.
*/ -int cap_settime(const struct timespec *ts, const struct timezone *tz) +int cap_settime(const struct timespec64 *ts, const struct timezone *tz) { if (!capable(CAP_SYS_TIME)) return -EPERM; diff --git a/security/security.c b/security/security.c index e81d5bb..0be5032 100644 --- a/security/security.c +++ b/security/security.c @@ -224,7 +224,7 @@ int security_syslog(int type) return security_ops->syslog(type); }
-int security_settime(const struct timespec *ts, const struct timezone *tz) +int security_settime64(const struct timespec64 *ts, const struct timezone *tz) { return security_ops->settime(ts, tz); } -- 1.7.9.5
Hi Arnd,
Please help to review the patch for introducing the security_settime64() function. Thanks a lot!
On Thursday 14 May 2015 15:03:10 Baolin Wang wrote:
This patch changes the "settime" pointer's argument with timespec64 type in security_operations structure, and also introduces the security_settime64() function with timespec64 type, meanwhile moves the security_settime() defined in security/security.c file to include/linux/security.h file as a static function.
Also introduces a default security_settime64() function with timespec64 type when it doesn't define the CONFIG_SECURITY macro. And change the cap_settime() function's argument with timespec64 type, which is only used by security_settime64() function.
Regarding the changelog: please start each patch changelog with a description of the current behavior and what is wrong with it, only then explain what you are doing to address it. You description above only says what you do, not why, and that is not too helpful for someone trying to dig through the git history later, or trying to review the patch out of context, for that matter.
Also, watch out for overly long lines. Try to make the subject no more than 60 characters (as explained before), and limit the text lines to about 70 characters.
The patch itself looks good to me, just one small improvement I'd do:
@@ -1790,7 +1791,13 @@ int security_capable_noaudit(const struct cred *cred, struct user_namespace *ns, int security_quotactl(int cmds, int type, int id, struct super_block *sb); int security_quota_on(struct dentry *dentry); int security_syslog(int type); -int security_settime(const struct timespec *ts, const struct timezone *tz); +int security_settime64(const struct timespec64 *ts, const struct timezone *tz); +static int security_settime(const struct timespec *ts, const struct timezone *tz) +{
- struct timespec64 ts64 = timespec_to_timespec64(*ts);
- return security_settime64(&ts64, tz);
+} int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); @@ -2040,10 +2047,18 @@ static inline int security_syslog(int type) return 0; } +static inline int security_settime64(const struct timespec64 *ts,
const struct timezone *tz)+{
- return cap_settime(ts, tz);
+}
static inline int security_settime(const struct timespec *ts, const struct timezone *tz) {
- return cap_settime(ts, tz);
- struct timsepc64 ts64 = timespec_to_timespec64(*ts);
- return security_settime64(&ts64, tz);
} static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
You have added two identical definitions for security_settime(). I would try to move that definition outside the #ifdef so you only need one of the two. If that ends up being ugly, I'd simplify the second definition to just call cap_settime() directly: security_settime64() is only a trivial wrapper here, so you can do the cap_settime() call in security_settime() and avoid the extra indirection (though the compiler won't care).
Arnd
On 18 May 2015 at 18:14, Arnd Bergmann arnd@linaro.org wrote:
On Thursday 14 May 2015 15:03:10 Baolin Wang wrote:
This patch changes the "settime" pointer's argument with timespec64 type
in security_operations
structure, and also introduces the security_settime64() function with
timespec64 type, meanwhile
moves the security_settime() defined in security/security.c file to
include/linux/security.h file
as a static function.
Also introduces a default security_settime64() function with timespec64
type when it doesn't define
the CONFIG_SECURITY macro. And change the cap_settime() function's
argument with timespec64 type,
which is only used by security_settime64() function.
Regarding the changelog: please start each patch changelog with a description of the current behavior and what is wrong with it, only then explain what you are doing to address it. You description above only says what you do, not why, and that is not too helpful for someone trying to dig through the git history later, or trying to review the patch out of context, for that matter.
Also, watch out for overly long lines. Try to make the subject no more than 60 characters (as explained before), and limit the text lines to about 70 characters.
The patch itself looks good to me, just one small improvement I'd do:
@@ -1790,7 +1791,13 @@ int security_capable_noaudit(const struct cred
*cred, struct user_namespace *ns,
int security_quotactl(int cmds, int type, int id, struct super_block
*sb);
int security_quota_on(struct dentry *dentry); int security_syslog(int type); -int security_settime(const struct timespec *ts, const struct timezone
*tz);
+int security_settime64(const struct timespec64 *ts, const struct
timezone *tz);
+static int security_settime(const struct timespec *ts, const struct
timezone *tz)
+{
struct timespec64 ts64 = timespec_to_timespec64(*ts);return security_settime64(&ts64, tz);+} int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); @@ -2040,10 +2047,18 @@ static inline int security_syslog(int type) return 0; }
+static inline int security_settime64(const struct timespec64 *ts,
const struct timezone *tz)+{
return cap_settime(ts, tz);+}
static inline int security_settime(const struct timespec *ts, const struct timezone *tz) {
return cap_settime(ts, tz);
struct timsepc64 ts64 = timespec_to_timespec64(*ts);return security_settime64(&ts64, tz);}
static inline int security_vm_enough_memory_mm(struct mm_struct *mm,
long pages)
You have added two identical definitions for security_settime(). I would try to move that definition outside the #ifdef so you only need one of the two. If that ends up being ugly, I'd simplify the second definition to just call cap_settime() directly: security_settime64() is only a trivial wrapper here, so you can do the cap_settime() call in security_settime() and avoid the extra indirection (though the compiler won't care).
Arnd
Thanks for your comments. And the simplification for the second definition of security_settime() is very reasonable, I'll simplify it. As well as modify the changelog to more helpful.
-
Arnd Bergmann -
Baolin Wang