Format specifier issue when building kernel

Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 343 bytes --]

I've been working off of Richard Guy Brigs git repo on branch
audit-for-next prepping my patch and I noticed a build warning:

kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
‘double’, but argument 3 has type ‘char *’ [-Wformat]

Looking at the code, it looks wrong:




-- 
Respectfully,

William C Roberts

[-- Attachment #1.2: Type: text/html, Size: 489 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 787 bytes --]

On Mon, Oct 28, 2013 at 4:30 PM, William Roberts
<bill.c.roberts@gmail.com>wrote:

> I've been working off of Richard Guy Brigs git repo on branch
> audit-for-next prepping my patch and I noticed a build warning:
>
> kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
> ‘double’, but argument 3 has type ‘char *’ [-Wformat]
>
> Looking at the code, it looks wrong:
>
>                                 audit_log_format(ab,
                                                 "
msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
                                                 (char *)data);

The issue appears on the % specifier in there, it picks it up as %.A, which
is of type double. Is this what was intended?


<snip>
>

-- 
Respectfully,

William C Roberts

[-- Attachment #1.2: Type: text/html, Size: 1703 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[Richard Guy Briggs]

On Mon, Oct 28, 2013 at 04:31:30PM -0700, William Roberts wrote:
> On Mon, Oct 28, 2013 at 4:30 PM, William Roberts
> <bill.c.roberts@gmail.com>wrote:
> 
> > I've been working off of Richard Guy Brigs git repo on branch
> > audit-for-next prepping my patch and I noticed a build warning:
> >
> > kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
> > ‘double’, but argument 3 has type ‘char *’ [-Wformat]
> >
> > Looking at the code, it looks wrong:
> >
> >                                 audit_log_format(ab,
>                                                  "
> msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
>                                                  (char *)data);
> 
> The issue appears on the % specifier in there, it picks it up as %.A, which
> is of type double. Is this what was intended?

Hmmm, that should have picked up a macro from 06051fbe in
audit-for-next.  It should be pre-processed to "%.8560s".

> William C Roberts


- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 1441 bytes --]

On Mon, Oct 28, 2013 at 6:35 PM, Richard Guy Briggs <rgb@redhat.com> wrote:

> On Mon, Oct 28, 2013 at 04:31:30PM -0700, William Roberts wrote:
> > On Mon, Oct 28, 2013 at 4:30 PM, William Roberts
> > <bill.c.roberts@gmail.com>wrote:
> >
> > > I've been working off of Richard Guy Brigs git repo on branch
> > > audit-for-next prepping my patch and I noticed a build warning:
> > >
> > > kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
> > > ‘double’, but argument 3 has type ‘char *’ [-Wformat]
> > >
> > > Looking at the code, it looks wrong:
> > >
> > >                                 audit_log_format(ab,
> >                                                  "
> > msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> >                                                  (char *)data);
> >
> > The issue appears on the % specifier in there, it picks it up as %.A,
> which
> > is of type double. Is this what was intended?
>
> Hmmm, that should have picked up a macro from 06051fbe in
> audit-for-next.  It should be pre-processed to "%.8560s".
>
> > William C Roberts
>
>
> - RGB
>
>
The qoutes are wrong for that.

> --
> Richard Guy Briggs <rbriggs@redhat.com>
> Senior Software Engineer
> Kernel Security
> AMER ENG Base Operating Systems
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635
> Internal: (81) 32635
> Alt: +1.613.693.0684x3545
>



-- 
Respectfully,

William C Roberts

[-- Attachment #1.2: Type: text/html, Size: 2487 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 2405 bytes --]

On Mon, Oct 28, 2013 at 6:43 PM, William Roberts
<bill.c.roberts@gmail.com>wrote:

>
>
>
> On Mon, Oct 28, 2013 at 6:35 PM, Richard Guy Briggs <rgb@redhat.com>wrote:
>
>> On Mon, Oct 28, 2013 at 04:31:30PM -0700, William Roberts wrote:
>> > On Mon, Oct 28, 2013 at 4:30 PM, William Roberts
>> > <bill.c.roberts@gmail.com>wrote:
>> >
>> > > I've been working off of Richard Guy Brigs git repo on branch
>> > > audit-for-next prepping my patch and I noticed a build warning:
>> > >
>> > > kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
>> > > ‘double’, but argument 3 has type ‘char *’ [-Wformat]
>> > >
>> > > Looking at the code, it looks wrong:
>> > >
>> > >                                 audit_log_format(ab,
>> >                                                  "
>> > msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
>> >                                                  (char *)data);
>> >
>> > The issue appears on the % specifier in there, it picks it up as %.A,
>> which
>> > is of type double. Is this what was intended?
>>
>> Hmmm, that should have picked up a macro from 06051fbe in
>> audit-for-next.  It should be pre-processed to "%.8560s".
>>
>> > William C Roberts
>>
>>
>> - RGB
>>
>>
> The qoutes are wrong for that.
>

Ok I see the value is not a string, but a numeric constant. Dont you need
to do something like this:
diff --git a/kernel/audit.c b/kernel/audit.c
index bf4b1af..81dde3d 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -825,10 +825,12 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)
                                if (err)
                                        break;
                        }
+#define STR_HELPER(x) #x
+#define STR(x) STR_HELPER(x)
                        audit_log_common_recv_msg(&ab, msg_type);
                        if (msg_type != AUDIT_USER_TTY)
                                audit_log_format(ab,
-                                                "
msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
+                                                "
msg='%."STR(AUDIT_MESSAGE_TEXT_MAX)"s'",
                                                 (char *)data);
                        else {
                                int size;


Unless their is some gnu-magic I don't know about.



> <snip>
>
>
>

-- 
Respectfully,

William C Roberts

[-- Attachment #1.2: Type: text/html, Size: 4228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[Richard Guy Briggs]

On Mon, Oct 28, 2013 at 08:55:08PM -0700, William Roberts wrote:
> On Mon, Oct 28, 2013 at 6:43 PM, William Roberts
> <bill.c.roberts@gmail.com>wrote:
> > On Mon, Oct 28, 2013 at 6:35 PM, Richard Guy Briggs <rgb@redhat.com>wrote:
> >> On Mon, Oct 28, 2013 at 04:31:30PM -0700, William Roberts wrote:
> >> > On Mon, Oct 28, 2013 at 4:30 PM, William Roberts
> >> > <bill.c.roberts@gmail.com>wrote:
> >> > > I've been working off of Richard Guy Brigs git repo on branch
> >> > > audit-for-next prepping my patch and I noticed a build warning:
> >> > >
> >> > > kernel/audit.c:832:8: warning: format ‘%A’ expects argument of type
> >> > > ‘double’, but argument 3 has type ‘char *’ [-Wformat]
> >> > >
> >> > > Looking at the code, it looks wrong:
> >> > >
> >> > >                                 audit_log_format(ab,
> >> >                                                  "
> >> > msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> >> >                                                  (char *)data);
> >> >
> >> > The issue appears on the % specifier in there, it picks it up as
> >> > %.A, which is of type double. Is this what was intended?
> >>
> >> Hmmm, that should have picked up a macro from 06051fbe in
> >> audit-for-next.  It should be pre-processed to "%.8560s".
> >>
> >> > William C Roberts
> >>
> >> - RGB
> >
> > The qoutes are wrong for that.
> 
> Ok I see the value is not a string, but a numeric constant. Dont you need
> to do something like this:
> diff --git a/kernel/audit.c b/kernel/audit.c
> index bf4b1af..81dde3d 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -825,10 +825,12 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)
>                                 if (err)
>                                         break;
>                         }
> +#define STR_HELPER(x) #x
> +#define STR(x) STR_HELPER(x)
>                         audit_log_common_recv_msg(&ab, msg_type);
>                         if (msg_type != AUDIT_USER_TTY)
>                                 audit_log_format(ab,
> -                                                "
> msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> +                                                "
> msg='%."STR(AUDIT_MESSAGE_TEXT_MAX)"s'",
>                                                  (char *)data);
>                         else {
>                                 int size;

Ugh.  That's not so easy to read...  Slightly longer, how about this?

diff --git a/kernel/audit.c b/kernel/audit.c
index 8378c5e..3f569d1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -824,11 +824,13 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 					break;
 			}
 			audit_log_common_recv_msg(&ab, msg_type);
-			if (msg_type != AUDIT_USER_TTY)
-				audit_log_format(ab,
-						 " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
-						 (char *)data);
-			else {
+			if (msg_type != AUDIT_USER_TTY) {
+				char fmt[64];
+				strcat(fmt, " msg='%.");
+				strcat(fmt, "AUDIT_MESSAGE_TEXT_MAX");
+				strcat(fmt, "s'");
+				audit_log_format(ab, fmt, (char *)data);
+			} else {
 				int size;
 
 				audit_log_format(ab, " data=");

> Unless their is some gnu-magic I don't know about.
> 
> William C Roberts

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 1482 bytes --]

<snip>

>
> Ugh.  That's not so easy to read...  Slightly longer, how about this?
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 8378c5e..3f569d1 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -824,11 +824,13 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)
>                                         break;
>                         }
>                         audit_log_common_recv_msg(&ab, msg_type);
> -                       if (msg_type != AUDIT_USER_TTY)
> -                               audit_log_format(ab,
> -                                                "
> msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> -                                                (char *)data);
> -                       else {
> +                       if (msg_type != AUDIT_USER_TTY) {
> +                               char fmt[64];
> +                               strcat(fmt, " msg='%.");
> +                               strcat(fmt, "AUDIT_MESSAGE_TEXT_MAX");
> +                               strcat(fmt, "s'");
> +                               audit_log_format(ab, fmt, (char *)data);
> +                       } else {
>                                 int size;
>
>                                 audit_log_format(ab, " data=");
>

I am ok with this. In fact I was going to do this the first time, but I
thought their would be some explicit reason to avoid the additional
run time overhead as the concat could be made at compile time.

<snip>

[-- Attachment #1.2: Type: text/html, Size: 2146 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[Richard Guy Briggs]

On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > +                       if (msg_type != AUDIT_USER_TTY) {
> > +                               char fmt[64];
> > +                               strcat(fmt, " msg='%.");
> > +                               strcat(fmt, "AUDIT_MESSAGE_TEXT_MAX");
> > +                               strcat(fmt, "s'");
> > +                               audit_log_format(ab, fmt, (char *)data);
> > +                       } else {
> 
> I am ok with this. In fact I was going to do this the first time, but I
> thought their would be some explicit reason to avoid the additional
> run time overhead as the concat could be made at compile time.

Ok, this was in danger of starting with fmt in an unknown state.  Latest
patch: 

diff --git a/kernel/audit.c b/kernel/audit.c
@@ -148,6 +148,8 @@ DEFINE_MUTEX(audit_cmd_mutex);
  * should be at least that large. */
 #define AUDIT_BUFSIZ 1024
 
+char usermsg_format[64] = "";
+
 /* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
  * audit_freelist.  Doing so eliminates many kmalloc/kfree calls. */
 #define AUDIT_MAXFREE  (2*NR_CPUS)
@@ -714,11 +716,15 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 					break;
 			}
 			audit_log_common_recv_msg(&ab, msg_type);
-			if (msg_type != AUDIT_USER_TTY)
-				audit_log_format(ab,
-						 " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
+			if (msg_type != AUDIT_USER_TTY) {
+				if (unlikely(usermsg_format[0] == 0))
+					snprintf(usermsg_format,
+						sizeof(usermsg_format),
+						" msg=\'%%.%ds\'", 
+						AUDIT_MESSAGE_TEXT_MAX);
+				audit_log_format(ab, usermsg_format,
 						 (char *)data);
-			else {
+			} else {
 				int size;
 
 				audit_log_format(ab, " data=");

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

Re: Format specifier issue when building kernel

[Steve Grubb]

On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > +                       if (msg_type != AUDIT_USER_TTY) {
> > > +                               char fmt[64];
> > > +                               strcat(fmt, " msg='%.");
> > > +                               strcat(fmt, "AUDIT_MESSAGE_TEXT_MAX");
> > > +                               strcat(fmt, "s'");
> > > +                               audit_log_format(ab, fmt, (char *)data);
> > > +                       } else {
> > 
> > I am ok with this. In fact I was going to do this the first time, but I
> > thought their would be some explicit reason to avoid the additional
> > run time overhead as the concat could be made at compile time.
> 
> Ok, this was in danger of starting with fmt in an unknown state.  Latest
> patch:
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> @@ -148,6 +148,8 @@ DEFINE_MUTEX(audit_cmd_mutex);
>   * should be at least that large. */
>  #define AUDIT_BUFSIZ 1024
> 
> +char usermsg_format[64] = "";

You might want this ^^^  to be static so its not global in scope.

-Steve


>  /* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
>   * audit_freelist.  Doing so eliminates many kmalloc/kfree calls. */
>  #define AUDIT_MAXFREE  (2*NR_CPUS)
> @@ -714,11 +716,15 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh) break;
>  			}
>  			audit_log_common_recv_msg(&ab, msg_type);
> -			if (msg_type != AUDIT_USER_TTY)
> -				audit_log_format(ab,
> -						 " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> +			if (msg_type != AUDIT_USER_TTY) {
> +				if (unlikely(usermsg_format[0] == 0))
> +					snprintf(usermsg_format,
> +						sizeof(usermsg_format),
> +						" msg=\'%%.%ds\'",
> +						AUDIT_MESSAGE_TEXT_MAX);
> +				audit_log_format(ab, usermsg_format,
>  						 (char *)data);
> -			else {
> +			} else {
>  				int size;
> 
>  				audit_log_format(ab, " data=");
> 
> - RGB
> 
> --
> Richard Guy Briggs <rbriggs@redhat.com>
> Senior Software Engineer
> Kernel Security
> AMER ENG Base Operating Systems
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635
> Internal: (81) 32635
> Alt: +1.613.693.0684x3545
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

Re: Format specifier issue when building kernel

[Richard Guy Briggs]

On Fri, Nov 01, 2013 at 12:34:55PM -0400, Steve Grubb wrote:
> On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> > On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > > +                       if (msg_type != AUDIT_USER_TTY) {
> > > > +                               char fmt[64];
> > > > +                               strcat(fmt, " msg='%.");
> > > > +                               strcat(fmt, "AUDIT_MESSAGE_TEXT_MAX");
> > > > +                               strcat(fmt, "s'");
> > > > +                               audit_log_format(ab, fmt, (char *)data);
> > > > +                       } else {
> > > 
> > > I am ok with this. In fact I was going to do this the first time, but I
> > > thought their would be some explicit reason to avoid the additional
> > > run time overhead as the concat could be made at compile time.
> > 
> > Ok, this was in danger of starting with fmt in an unknown state.  Latest
> > patch:
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > @@ -148,6 +148,8 @@ DEFINE_MUTEX(audit_cmd_mutex);
> >   * should be at least that large. */
> >  #define AUDIT_BUFSIZ 1024
> > 
> > +char usermsg_format[64] = "";
> 
> You might want this ^^^  to be static so its not global in scope.

Yup, good point.  Thanks.

> -Steve
> 
> >  /* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
> >   * audit_freelist.  Doing so eliminates many kmalloc/kfree calls. */
> >  #define AUDIT_MAXFREE  (2*NR_CPUS)
> > @@ -714,11 +716,15 @@ static int audit_receive_msg(struct sk_buff *skb,
> > struct nlmsghdr *nlh) break;
> >  			}
> >  			audit_log_common_recv_msg(&ab, msg_type);
> > -			if (msg_type != AUDIT_USER_TTY)
> > -				audit_log_format(ab,
> > -						 " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> > +			if (msg_type != AUDIT_USER_TTY) {
> > +				if (unlikely(usermsg_format[0] == 0))
> > +					snprintf(usermsg_format,
> > +						sizeof(usermsg_format),
> > +						" msg=\'%%.%ds\'",
> > +						AUDIT_MESSAGE_TEXT_MAX);
> > +				audit_log_format(ab, usermsg_format,
> >  						 (char *)data);
> > -			else {
> > +			} else {
> >  				int size;
> > 
> >  				audit_log_format(ab, " data=");
> > 
> > - RGB
> > 
> > --
> > Richard Guy Briggs <rbriggs@redhat.com>
> > Senior Software Engineer
> > Kernel Security
> > AMER ENG Base Operating Systems
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635
> > Internal: (81) 32635
> > Alt: +1.613.693.0684x3545
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> 

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

Re: Format specifier issue when building kernel

[Richard Guy Briggs]

On Fri, Nov 01, 2013 at 12:38:15PM -0400, Richard Guy Briggs wrote:
> On Fri, Nov 01, 2013 at 12:34:55PM -0400, Steve Grubb wrote:
> > On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> > > On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > +char usermsg_format[64] = "";
> > 
> > You might want this ^^^  to be static so its not global in scope.
> 
> Yup, good point.  Thanks.

Better yet, make it local to that if statement, but Eric Paris has a far
more readable and elegant solution:

diff --git a/kernel/audit.c b/kernel/audit.c
@@ -864,8 +864,8 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)
                        }
                        audit_log_common_recv_msg(&ab, msg_type);
                        if (msg_type != AUDIT_USER_TTY)
-                               audit_log_format(ab,
-                                                " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
+                               audit_log_format(ab, " msg='%.*s'",
+                                                AUDIT_MESSAGE_TEXT_MAX,
                                                 (char *)data);
                        else {
                                int size;

I forgot about the * format specifier...

> > -Steve
> > 
> > > - RGB
> 
> - RGB

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: +1.647.777.2635
Internal: (81) 32635
Alt: +1.613.693.0684x3545

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 1835 bytes --]

On Nov 1, 2013 12:58 PM, "Richard Guy Briggs" <rgb@redhat.com> wrote:
>
> On Fri, Nov 01, 2013 at 12:38:15PM -0400, Richard Guy Briggs wrote:
> > On Fri, Nov 01, 2013 at 12:34:55PM -0400, Steve Grubb wrote:
> > > On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> > > > On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > > +char usermsg_format[64] = "";
> > >
> > > You might want this ^^^  to be static so its not global in scope.
> >
> > Yup, good point.  Thanks.
>
> Better yet, make it local to that if statement, but Eric Paris has a far
> more readable and elegant solution:
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> @@ -864,8 +864,8 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)
>                         }
>                         audit_log_common_recv_msg(&ab, msg_type);
>                         if (msg_type != AUDIT_USER_TTY)
> -                               audit_log_format(ab,
> -                                                "
msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> +                               audit_log_format(ab, " msg='%.*s'",
> +                                                AUDIT_MESSAGE_TEXT_MAX,
>                                                  (char *)data);
>                         else {
>                                 int size;
>
> I forgot about the * format specifier...
Yeah me too... I think everyone does :-)
>
> > > -Steve
> > >
> > > > - RGB
> >
> > - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <rbriggs@redhat.com>
> Senior Software Engineer
> Kernel Security
> AMER ENG Base Operating Systems
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635
> Internal: (81) 32635
> Alt: +1.613.693.0684x3545
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Type: text/html, Size: 2616 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[David Rebman]

[-- Attachment #1.1: Type: text/plain, Size: 2354 bytes --]

unsubscribe

From: William Roberts <bill.c.roberts@gmail.com<mailto:bill.c.roberts@gmail.com>>
Date: Friday, November 1, 2013 at 5:10 PM
To: Richard Guy Briggs <rgb@redhat.com<mailto:rgb@redhat.com>>
Cc: "linux-audit@redhat.com<mailto:linux-audit@redhat.com>" <linux-audit@redhat.com<mailto:linux-audit@redhat.com>>
Subject: Re: Format specifier issue when building kernel


On Nov 1, 2013 12:58 PM, "Richard Guy Briggs" <rgb@redhat.com<mailto:rgb@redhat.com>> wrote:
>
> On Fri, Nov 01, 2013 at 12:38:15PM -0400, Richard Guy Briggs wrote:
> > On Fri, Nov 01, 2013 at 12:34:55PM -0400, Steve Grubb wrote:
> > > On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> > > > On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > > +char usermsg_format[64] = "";
> > >
> > > You might want this ^^^  to be static so its not global in scope.
> >
> > Yup, good point.  Thanks.
>
> Better yet, make it local to that if statement, but Eric Paris has a far
> more readable and elegant solution:
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> @@ -864,8 +864,8 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)
>                         }
>                         audit_log_common_recv_msg(&ab, msg_type);
>                         if (msg_type != AUDIT_USER_TTY)
> -                               audit_log_format(ab,
> -                                                " msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> +                               audit_log_format(ab, " msg='%.*s'",
> +                                                AUDIT_MESSAGE_TEXT_MAX,
>                                                  (char *)data);
>                         else {
>                                 int size;
>
> I forgot about the * format specifier...
Yeah me too... I think everyone does :-)
>
> > > -Steve
> > >
> > > > - RGB
> >
> > - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <rbriggs@redhat.com<mailto:rbriggs@redhat.com>>
> Senior Software Engineer
> Kernel Security
> AMER ENG Base Operating Systems
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635
> Internal: (81) 32635
> Alt: +1.613.693.0684x3545
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com<mailto:Linux-audit@redhat.com>
> https://www.redhat.com/mailman/listinfo/linux-audit

[-- Attachment #1.2: Type: text/html, Size: 4828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Format specifier issue when building kernel

[William Roberts]

[-- Attachment #1.1: Type: text/plain, Size: 2344 bytes --]

FYI... Sending unsubscribe to this list doest work.
On Nov 1, 2013 3:20 PM, "David Rebman" <drebman@nexuminc.com> wrote:

>  unsubscribe
>
>   From: William Roberts <bill.c.roberts@gmail.com>
> Date: Friday, November 1, 2013 at 5:10 PM
> To: Richard Guy Briggs <rgb@redhat.com>
> Cc: "linux-audit@redhat.com" <linux-audit@redhat.com>
> Subject: Re: Format specifier issue when building kernel
>
>
> On Nov 1, 2013 12:58 PM, "Richard Guy Briggs" <rgb@redhat.com> wrote:
> >
> > On Fri, Nov 01, 2013 at 12:38:15PM -0400, Richard Guy Briggs wrote:
> > > On Fri, Nov 01, 2013 at 12:34:55PM -0400, Steve Grubb wrote:
> > > > On Friday, November 01, 2013 12:24:55 PM Richard Guy Briggs wrote:
> > > > > On Thu, Oct 31, 2013 at 12:25:55PM -0700, William Roberts wrote:
> > > > > +char usermsg_format[64] = "";
> > > >
> > > > You might want this ^^^  to be static so its not global in scope.
> > >
> > > Yup, good point.  Thanks.
> >
> > Better yet, make it local to that if statement, but Eric Paris has a far
> > more readable and elegant solution:
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > @@ -864,8 +864,8 @@ static int audit_receive_msg(struct sk_buff *skb,
> > struct nlmsghdr *nlh)
> >                         }
> >                         audit_log_common_recv_msg(&ab, msg_type);
> >                         if (msg_type != AUDIT_USER_TTY)
> > -                               audit_log_format(ab,
> > -                                                "
> msg='%.AUDIT_MESSAGE_TEXT_MAXs'",
> > +                               audit_log_format(ab, " msg='%.*s'",
> > +                                                AUDIT_MESSAGE_TEXT_MAX,
> >                                                  (char *)data);
> >                         else {
> >                                 int size;
> >
> > I forgot about the * format specifier...
> Yeah me too... I think everyone does :-)
> >
> > > > -Steve
> > > >
> > > > > - RGB
> > >
> > > - RGB
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rbriggs@redhat.com>
> > Senior Software Engineer
> > Kernel Security
> > AMER ENG Base Operating Systems
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635
> > Internal: (81) 32635
> > Alt: +1.613.693.0684x3545
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
>

[-- Attachment #1.2: Type: text/html, Size: 4379 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]