don't reallocate buffer in every audit_sockaddr()
PATCH 1/15 don't reallocate buffer in every audit_sockaddr()
No need to do that more than once per process lifetime; allocating/freeing on each sendto/accept/etc. is bloody pointless.
--- kernel/auditsc.c | 46 ++++++++++++++++++++++------------------------ 1 files changed, 22 insertions(+), 24 deletions(-)
kernel/auditsc.c +22 -24
@@ -162,12 +162,6 @@ struct audit_aux_data_socketcall {unsigned long args[0 ]; }; -struct audit_aux_data_sockaddr { -struct audit_aux_data d; -int len; -char a[0 ]; -}; -struct audit_aux_data_fd_pair {struct audit_aux_data d;int fd[2 ];
@@ -208,7 +202,8 @@ struct audit_context {struct audit_context *previous;/* For nested syscalls */ struct audit_aux_data *aux;struct audit_aux_data *aux_pids; - +struct sockaddr_storage *sockaddr; +size_t sockaddr_len;/* Save things to print about task_struct */ pid_t pid, ppid;uid_t uid, euid, suid, fsuid;
@@ -891,6 +886,7 @@ static inline void audit_free_context(struct audit_context *context)free_tree_refs (context);audit_free_aux (context);kfree (context->filterkey); +kfree (context->sockaddr);kfree (context); context = previous; }while (context);
@@ -1322,13 +1318,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_format (ab," a%d=%lx" , i, axs->args[i]);break ; } -case AUDIT_SOCKADDR : { -struct audit_aux_data_sockaddr *axs = (void *)aux; - -audit_log_format (ab,"saddr=" ); -audit_log_n_hex (ab, axs->a, axs->len); -break ; } -case AUDIT_FD_PAIR : {struct audit_aux_data_fd_pair *axs = (void *)aux;audit_log_format (ab,"fd0=%d fd1=%d" , axs->fd[0 ], axs->fd[1 ]);
@@ -1338,6 +1327,16 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_end (ab); } +if (context->sockaddr_len) { + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_SOCKADDR ); +if (ab) { +audit_log_format (ab,"saddr=" ); +audit_log_n_hex (ab, (void *)context->sockaddr, + context->sockaddr_len); +audit_log_end (ab); + } + } +for (aux = context->aux_pids; aux; aux = aux->next) {struct audit_aux_data_pids *axs = (void *)aux;
@@ -1604,6 +1603,7 @@ void audit_syscall_exit(int valid, long return_code) context->aux_pids =NULL ; context->target_pid =0 ; context->target_sid =0 ; + context->sockaddr_len =0 ;kfree (context->filterkey); context->filterkey =NULL ; tsk->audit_context = context;
@@ -2354,22 +2354,20 @@ int __audit_fd_pair(int fd1, int fd2)*/ int audit_sockaddr (int len,void *a) { -struct audit_aux_data_sockaddr *ax;struct audit_context *context = current->audit_context;if (likely (!context || context->dummy))return 0 ; - ax =kmalloc (sizeof (*ax) + len,GFP_KERNEL ); -if (!ax) -return -ENOMEM ; - - ax->len = len; -memcpy (ax->a, a, len); +if (!context->sockaddr) { +void *p =kmalloc (sizeof (struct sockaddr_storage ),GFP_KERNEL ); +if (!p) +return -ENOMEM ; + context->sockaddr = p; + } - ax->d.type =AUDIT_SOCKADDR ; - ax->d.next = context->aux; - context->aux = (void *)ax; + context->sockaddr_len = len; +memcpy (context->sockaddr, a, len);return 0 ; }
Signature · 1 line
1.5.6.5
Quoted 5 lines · Al Viro
No need to do that more than once per process lifetime; allocating/freeing on each sendto/accept/etc. is bloody pointless.
Signed-off-by Al Viro <[email protected]>
Quoted 119 lines
--- kernel/auditsc.c | 46 ++++++++++++++++++++++------------------------ 1 files changed, 22 insertions(+), 24 deletions(-)kernel/auditsc.c +22
-24@@ -162,12 +162,6 @@ struct audit_aux_data_socketcall {unsigned long args[0 ]; }; -struct audit_aux_data_sockaddr { -struct audit_aux_data d; -int len; -char a[0 ]; -}; -struct audit_aux_data_fd_pair {struct audit_aux_data d;int fd[2 ];@@ -208,7 +202,8 @@ struct audit_context {struct audit_context *previous;/* For nested syscalls */ struct audit_aux_data *aux;struct audit_aux_data *aux_pids; - +struct sockaddr_storage *sockaddr; +size_t sockaddr_len;/* Save things to print about task_struct */ pid_t pid, ppid;uid_t uid, euid, suid, fsuid;@@ -891,6 +886,7 @@ static inline void audit_free_context(struct audit_context *context)free_tree_refs (context);audit_free_aux (context);kfree (context->filterkey); +kfree (context->sockaddr);kfree (context); context = previous; }while (context);@@ -1322,13 +1318,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_format (ab," a%d=%lx" , i, axs->args[i]);break ; } -case AUDIT_SOCKADDR : { -struct audit_aux_data_sockaddr *axs = (void *)aux; - -audit_log_format (ab,"saddr=" ); -audit_log_n_hex (ab, axs->a, axs->len); -break ; } -case AUDIT_FD_PAIR : {struct audit_aux_data_fd_pair *axs = (void *)aux;audit_log_format (ab,"fd0=%d fd1=%d" , axs->fd[0 ], axs->fd[1 ]);@@ -1338,6 +1327,16 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_end (ab); } +if (context->sockaddr_len) { + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_SOCKADDR ); +if (ab) { +audit_log_format (ab,"saddr=" ); +audit_log_n_hex (ab, (void *)context->sockaddr, + context->sockaddr_len); +audit_log_end (ab); + } + } +for (aux = context->aux_pids; aux; aux = aux->next) {struct audit_aux_data_pids *axs = (void *)aux;@@ -1604,6 +1603,7 @@ void audit_syscall_exit(int valid, long return_code) context->aux_pids =NULL ; context->target_pid =0 ; context->target_sid =0 ; + context->sockaddr_len =0 ;kfree (context->filterkey); context->filterkey =NULL ; tsk->audit_context = context;@@ -2354,22 +2354,20 @@ int __audit_fd_pair(int fd1, int fd2)*/ int audit_sockaddr (int len,void *a) { -struct audit_aux_data_sockaddr *ax;struct audit_context *context = current->audit_context;if (likely (!context || context->dummy))return 0 ; - ax =kmalloc (sizeof (*ax) + len,GFP_KERNEL ); -if (!ax) -return -ENOMEM ; - - ax->len = len; -memcpy (ax->a, a, len); +if (!context->sockaddr) { +void *p =kmalloc (sizeof (struct sockaddr_storage ),GFP_KERNEL ); +if (!p) +return -ENOMEM ; + context->sockaddr = p; + } - ax->d.type =AUDIT_SOCKADDR ; - ax->d.next = context->aux; - context->aux = (void *)ax; + context->sockaddr_len = len; +memcpy (context->sockaddr, a, len);return 0 ; }Signature · 8 lines
1.5.6.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 2 lines
James Morris <[email protected]>
Quoted 6 lines · On Wed, 17 Dec 2008 05:11:10 +0000 Al Viro <[email protected]> wrote:
int
audit_sockaddr(int len, void *a)
{kernel/auditsc.c +0
-1-struct audit_aux_data_sockaddr *ax;struct audit_context *context = current->audit_context;if (likely (!context || context->dummy))
kernel/auditsc.c int __audit_fd_pair(int fd1, int fd2) · line 2363
Quoted 10 lines
kernel/auditsc.c +2
-6return 0 ; - ax =kmalloc (sizeof (*ax) + len,GFP_KERNEL ); -if (!ax) -return -ENOMEM ; - - ax->len = len; -memcpy (ax->a, a, len); +if (!context->sockaddr) { +void *p =kmalloc (sizeof (struct sockaddr_storage ),GFP_KERNEL );
argh, I really hate having to run all around the code verifying that the type passed to sizeof matches the type that we'll be storing there :(
kernel/auditsc.c int __audit_fd_pair(int fd1, int fd2) · line 2372
Quoted 12 lines
kernel/auditsc.c +6
-3+if (!p) +return -ENOMEM ; + context->sockaddr = p; + } - ax->d.type =AUDIT_SOCKADDR ; - ax->d.next = context->aux; - context->aux = (void *)ax; + context->sockaddr_len = len; +memcpy (context->sockaddr, a, len);return 0 ; }
stoopid question: can an audit_contect be shared between threads/processes? If so, is locking needed around the read/test/write of context->sockaddr and friends?
Quoted 5 lines · Andrew Morton
kernel/auditsc.c int __audit_fd_pair(int fd1, int fd2) · line 2363
Quoted 2 lines
kernel/auditsc.c +2
-0+if (!context->sockaddr) { +void *p =kmalloc (sizeof (struct sockaddr_storage ),GFP_KERNEL );argh, I really hate having to run all around the code verifying that the type passed to sizeof matches the type that we'll be storing there :(
And I really hate being unable to find the places doing such allocations without serious parsing. Matter of taste.
Quoted 3 lines
stoopid question: can an
audit_contectbe shared between threads/processes? If so, is locking needed around the read/test/write ofcontext->sockaddrand friends?
It can't. And that's very fortunate, since otherwise all that crap would
a) be horribly racy
b) require shitloads of locking all over the place, driving the
overhead even higher.