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]>
---
kernel/auditsc.c | 46 ++++++++++++++++++++++------------------------
1 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 2a3f0af..aca9ddb 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -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 *ts
audit_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 *ts
audit_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;
}
--
1.5.6.5
On Wed, 17 Dec 2008, Al Viro wrote:
>
> 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]>
Reviewed-by: James Morris <[email protected]>
> ---
> kernel/auditsc.c | 46 ++++++++++++++++++++++------------------------
> 1 files changed, 22 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 2a3f0af..aca9ddb 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -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 *ts
> audit_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 *ts
> audit_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;
> }
>
> --
> 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/
>
--
James Morris
<[email protected]>
On Wed, 17 Dec 2008 05:11:10 +0000 Al Viro <[email protected]> wrote:
> 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);
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 :(
> + 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?
On Tue, Dec 16, 2008 at 11:49:27PM -0800, Andrew Morton wrote:
> > + 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.
> 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?
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.