* [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 9:30 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 9:30 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML; +Cc: Richard Guy Briggs If there is a memory allocation error when trying to change an audit kernel feature value, the ignored allocation error will trigger a NULL pointer dereference oops on subsequent use of that pointer. Return instead. Passes audit-testsuite. See: https://github.com/linux-audit/audit-kernel/issues/76 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/audit.c b/kernel/audit.c index 5c25449..2de74be 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature return; ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); + if (!ab) + return; audit_log_task_info(ab, current); audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", audit_feature_names[which], !!old_feature, !!new_feature, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 9:30 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 9:30 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML Cc: Paul Moore, Eric Paris, Steve Grubb, Richard Guy Briggs If there is a memory allocation error when trying to change an audit kernel feature value, the ignored allocation error will trigger a NULL pointer dereference oops on subsequent use of that pointer. Return instead. Passes audit-testsuite. See: https://github.com/linux-audit/audit-kernel/issues/76 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/audit.c b/kernel/audit.c index 5c25449..2de74be 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature return; ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); + if (!ab) + return; audit_log_task_info(ab, current); audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", audit_feature_names[which], !!old_feature, !!new_feature, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference 2018-02-21 9:30 ` Richard Guy Briggs @ 2018-02-21 23:49 ` Paul Moore -1 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2018-02-21 23:49 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Linux-Audit Mailing List, LKML On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > If there is a memory allocation error when trying to change an audit > kernel feature value, the ignored allocation error will trigger a NULL > pointer dereference oops on subsequent use of that pointer. Return > instead. > > Passes audit-testsuite. > See: https://github.com/linux-audit/audit-kernel/issues/76 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > kernel/audit.c | 2 ++ > 1 file changed, 2 insertions(+) Thanks, merged. In the future a "[PATCH v2]" prefix would be appreciated for patches like this, it makes things a little easier in my inbox. > diff --git a/kernel/audit.c b/kernel/audit.c > index 5c25449..2de74be 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > return; > > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > + if (!ab) > + return; > audit_log_task_info(ab, current); > audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > audit_feature_names[which], !!old_feature, !!new_feature, > -- > 1.8.3.1 -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 23:49 ` Paul Moore 0 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2018-02-21 23:49 UTC (permalink / raw) To: Richard Guy Briggs Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > If there is a memory allocation error when trying to change an audit > kernel feature value, the ignored allocation error will trigger a NULL > pointer dereference oops on subsequent use of that pointer. Return > instead. > > Passes audit-testsuite. > See: https://github.com/linux-audit/audit-kernel/issues/76 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > kernel/audit.c | 2 ++ > 1 file changed, 2 insertions(+) Thanks, merged. In the future a "[PATCH v2]" prefix would be appreciated for patches like this, it makes things a little easier in my inbox. > diff --git a/kernel/audit.c b/kernel/audit.c > index 5c25449..2de74be 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > return; > > ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > + if (!ab) > + return; > audit_log_task_info(ab, current); > audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > audit_feature_names[which], !!old_feature, !!new_feature, > -- > 1.8.3.1 -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference 2018-02-21 23:49 ` Paul Moore @ 2018-02-22 0:02 ` Paul Moore -1 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2018-02-22 0:02 UTC (permalink / raw) To: Richard Guy Briggs; +Cc: Linux-Audit Mailing List, LKML On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> wrote: > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: >> If there is a memory allocation error when trying to change an audit >> kernel feature value, the ignored allocation error will trigger a NULL >> pointer dereference oops on subsequent use of that pointer. Return >> instead. >> >> Passes audit-testsuite. >> See: https://github.com/linux-audit/audit-kernel/issues/76 >> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> >> --- >> kernel/audit.c | 2 ++ >> 1 file changed, 2 insertions(+) > > Thanks, merged. > > In the future a "[PATCH v2]" prefix would be appreciated for patches > like this, it makes things a little easier in my inbox. After merging this I went through all the other callers to see if they suffered the same mistake and everyone except for IMA was checking the returned pointer for NULL. Upon looking at the IMA code, and the audit code which is called, I realized we are actually "ok" as audit_log_task_info(), audit_log_format(), audit_log_end(), and others all check for a NULL audit_buffer at the very top of the functions. I'm going to leave this patch merged, it's a good practice after all, but I don't believe that unpatched systems are in any danger of oops'ing here. >> diff --git a/kernel/audit.c b/kernel/audit.c >> index 5c25449..2de74be 100644 >> --- a/kernel/audit.c >> +++ b/kernel/audit.c >> @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature >> return; >> >> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); >> + if (!ab) >> + return; >> audit_log_task_info(ab, current); >> audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", >> audit_feature_names[which], !!old_feature, !!new_feature, >> -- >> 1.8.3.1 -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-22 0:02 ` Paul Moore 0 siblings, 0 replies; 14+ messages in thread From: Paul Moore @ 2018-02-22 0:02 UTC (permalink / raw) To: Richard Guy Briggs Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> wrote: > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: >> If there is a memory allocation error when trying to change an audit >> kernel feature value, the ignored allocation error will trigger a NULL >> pointer dereference oops on subsequent use of that pointer. Return >> instead. >> >> Passes audit-testsuite. >> See: https://github.com/linux-audit/audit-kernel/issues/76 >> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> >> --- >> kernel/audit.c | 2 ++ >> 1 file changed, 2 insertions(+) > > Thanks, merged. > > In the future a "[PATCH v2]" prefix would be appreciated for patches > like this, it makes things a little easier in my inbox. After merging this I went through all the other callers to see if they suffered the same mistake and everyone except for IMA was checking the returned pointer for NULL. Upon looking at the IMA code, and the audit code which is called, I realized we are actually "ok" as audit_log_task_info(), audit_log_format(), audit_log_end(), and others all check for a NULL audit_buffer at the very top of the functions. I'm going to leave this patch merged, it's a good practice after all, but I don't believe that unpatched systems are in any danger of oops'ing here. >> diff --git a/kernel/audit.c b/kernel/audit.c >> index 5c25449..2de74be 100644 >> --- a/kernel/audit.c >> +++ b/kernel/audit.c >> @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature >> return; >> >> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); >> + if (!ab) >> + return; >> audit_log_task_info(ab, current); >> audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", >> audit_feature_names[which], !!old_feature, !!new_feature, >> -- >> 1.8.3.1 -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference 2018-02-22 0:02 ` Paul Moore @ 2018-02-22 0:34 ` Eric Paris -1 siblings, 0 replies; 14+ messages in thread From: Eric Paris @ 2018-02-22 0:34 UTC (permalink / raw) To: Paul Moore, Richard Guy Briggs; +Cc: Linux-Audit Mailing List, LKML I think if we went back and looked at history we'd see that all of the code originally had none of the if(!ab) checks after allocation and they just sorta slowly crept in over time. I prefer this pattern, but it used to be the opposite everywhere. On Wed, 2018-02-21 at 19:02 -0500, Paul Moore wrote: > On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> > wrote: > > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com > > > wrote: > > > If there is a memory allocation error when trying to change an > > > audit > > > kernel feature value, the ignored allocation error will trigger a > > > NULL > > > pointer dereference oops on subsequent use of that > > > pointer. Return > > > instead. > > > > > > Passes audit-testsuite. > > > See: https://github.com/linux-audit/audit-kernel/issues/76 > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > kernel/audit.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > Thanks, merged. > > > > In the future a "[PATCH v2]" prefix would be appreciated for > > patches > > like this, it makes things a little easier in my inbox. > > After merging this I went through all the other callers to see if > they > suffered the same mistake and everyone except for IMA was checking > the > returned pointer for NULL. Upon looking at the IMA code, and the > audit code which is called, I realized we are actually "ok" as > audit_log_task_info(), audit_log_format(), audit_log_end(), and > others > all check for a NULL audit_buffer at the very top of the functions. > I'm going to leave this patch merged, it's a good practice after all, > but I don't believe that unpatched systems are in any danger of > oops'ing here. > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index 5c25449..2de74be 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int > > > which, u32 old_feature, u32 new_feature > > > return; > > > > > > ab = audit_log_start(NULL, GFP_KERNEL, > > > AUDIT_FEATURE_CHANGE); > > > + if (!ab) > > > + return; > > > audit_log_task_info(ab, current); > > > audit_log_format(ab, " feature=%s old=%u new=%u > > > old_lock=%u new_lock=%u res=%d", > > > audit_feature_names[which], > > > !!old_feature, !!new_feature, > > > -- > > > 1.8.3.1 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-22 0:34 ` Eric Paris 0 siblings, 0 replies; 14+ messages in thread From: Eric Paris @ 2018-02-22 0:34 UTC (permalink / raw) To: Paul Moore, Richard Guy Briggs Cc: Linux-Audit Mailing List, LKML, Steve Grubb I think if we went back and looked at history we'd see that all of the code originally had none of the if(!ab) checks after allocation and they just sorta slowly crept in over time. I prefer this pattern, but it used to be the opposite everywhere. On Wed, 2018-02-21 at 19:02 -0500, Paul Moore wrote: > On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> > wrote: > > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com > > > wrote: > > > If there is a memory allocation error when trying to change an > > > audit > > > kernel feature value, the ignored allocation error will trigger a > > > NULL > > > pointer dereference oops on subsequent use of that > > > pointer. Return > > > instead. > > > > > > Passes audit-testsuite. > > > See: https://github.com/linux-audit/audit-kernel/issues/76 > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > kernel/audit.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > Thanks, merged. > > > > In the future a "[PATCH v2]" prefix would be appreciated for > > patches > > like this, it makes things a little easier in my inbox. > > After merging this I went through all the other callers to see if > they > suffered the same mistake and everyone except for IMA was checking > the > returned pointer for NULL. Upon looking at the IMA code, and the > audit code which is called, I realized we are actually "ok" as > audit_log_task_info(), audit_log_format(), audit_log_end(), and > others > all check for a NULL audit_buffer at the very top of the functions. > I'm going to leave this patch merged, it's a good practice after all, > but I don't believe that unpatched systems are in any danger of > oops'ing here. > > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index 5c25449..2de74be 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int > > > which, u32 old_feature, u32 new_feature > > > return; > > > > > > ab = audit_log_start(NULL, GFP_KERNEL, > > > AUDIT_FEATURE_CHANGE); > > > + if (!ab) > > > + return; > > > audit_log_task_info(ab, current); > > > audit_log_format(ab, " feature=%s old=%u new=%u > > > old_lock=%u new_lock=%u res=%d", > > > audit_feature_names[which], > > > !!old_feature, !!new_feature, > > > -- > > > 1.8.3.1 > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference 2018-02-22 0:02 ` Paul Moore @ 2018-02-22 0:55 ` Richard Guy Briggs -1 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-22 0:55 UTC (permalink / raw) To: Paul Moore; +Cc: Linux-Audit Mailing List, LKML On 2018-02-21 19:02, Paul Moore wrote: > On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > >> If there is a memory allocation error when trying to change an audit > >> kernel feature value, the ignored allocation error will trigger a NULL > >> pointer dereference oops on subsequent use of that pointer. Return > >> instead. > >> > >> Passes audit-testsuite. > >> See: https://github.com/linux-audit/audit-kernel/issues/76 > >> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > >> --- > >> kernel/audit.c | 2 ++ > >> 1 file changed, 2 insertions(+) > > > > Thanks, merged. > > > > In the future a "[PATCH v2]" prefix would be appreciated for patches > > like this, it makes things a little easier in my inbox. (Sorry, forgot in haste to get that fixed one out...) > After merging this I went through all the other callers to see if they > suffered the same mistake and everyone except for IMA was checking the > returned pointer for NULL. Upon looking at the IMA code, and the > audit code which is called, I realized we are actually "ok" as > audit_log_task_info(), audit_log_format(), audit_log_end(), and others > all check for a NULL audit_buffer at the very top of the functions. > I'm going to leave this patch merged, it's a good practice after all, > but I don't believe that unpatched systems are in any danger of > oops'ing here. On review, agreed. My ctags/cscope DBs need regeneration, so I hadn't noticed that the functions to which I was led weren't the ones I was seeking, and while these three do check, not all functions that accept a struct audit_buffer pointer parameter don't check for NULL. Now that I check, I only find audit_expand (whose callers are all protected) and audit_log_d_path (whose callers all appear to be protected), the latter of which I've spent a bit of time staring at of late (ghak8, ghak21...). We're ok. > >> diff --git a/kernel/audit.c b/kernel/audit.c > >> index 5c25449..2de74be 100644 > >> --- a/kernel/audit.c > >> +++ b/kernel/audit.c > >> @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > >> return; > >> > >> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > >> + if (!ab) > >> + return; > >> audit_log_task_info(ab, current); > >> audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > >> audit_feature_names[which], !!old_feature, !!new_feature, > >> -- > >> 1.8.3.1 > > -- > paul moore > www.paul-moore.com - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-22 0:55 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-22 0:55 UTC (permalink / raw) To: Paul Moore; +Cc: Linux-Audit Mailing List, LKML, Eric Paris, Steve Grubb On 2018-02-21 19:02, Paul Moore wrote: > On Wed, Feb 21, 2018 at 6:49 PM, Paul Moore <paul@paul-moore.com> wrote: > > On Wed, Feb 21, 2018 at 4:30 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > >> If there is a memory allocation error when trying to change an audit > >> kernel feature value, the ignored allocation error will trigger a NULL > >> pointer dereference oops on subsequent use of that pointer. Return > >> instead. > >> > >> Passes audit-testsuite. > >> See: https://github.com/linux-audit/audit-kernel/issues/76 > >> Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > >> --- > >> kernel/audit.c | 2 ++ > >> 1 file changed, 2 insertions(+) > > > > Thanks, merged. > > > > In the future a "[PATCH v2]" prefix would be appreciated for patches > > like this, it makes things a little easier in my inbox. (Sorry, forgot in haste to get that fixed one out...) > After merging this I went through all the other callers to see if they > suffered the same mistake and everyone except for IMA was checking the > returned pointer for NULL. Upon looking at the IMA code, and the > audit code which is called, I realized we are actually "ok" as > audit_log_task_info(), audit_log_format(), audit_log_end(), and others > all check for a NULL audit_buffer at the very top of the functions. > I'm going to leave this patch merged, it's a good practice after all, > but I don't believe that unpatched systems are in any danger of > oops'ing here. On review, agreed. My ctags/cscope DBs need regeneration, so I hadn't noticed that the functions to which I was led weren't the ones I was seeking, and while these three do check, not all functions that accept a struct audit_buffer pointer parameter don't check for NULL. Now that I check, I only find audit_expand (whose callers are all protected) and audit_log_d_path (whose callers all appear to be protected), the latter of which I've spent a bit of time staring at of late (ghak8, ghak21...). We're ok. > >> diff --git a/kernel/audit.c b/kernel/audit.c > >> index 5c25449..2de74be 100644 > >> --- a/kernel/audit.c > >> +++ b/kernel/audit.c > >> @@ -1059,6 +1059,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > >> return; > >> > >> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > >> + if (!ab) > >> + return; > >> audit_log_task_info(ab, current); > >> audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > >> audit_feature_names[which], !!old_feature, !!new_feature, > >> -- > >> 1.8.3.1 > > -- > paul moore > www.paul-moore.com - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 6:47 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 6:47 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML; +Cc: Richard Guy Briggs If there is a memory allocation error when trying to change an audit kernel feature value, the ignored allocation error will trigger a NULL pointer dereference oops on subsequent use of that pointer. Return instead. See: https://github.com/linux-audit/audit-kernel/issues/76 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/audit.c b/kernel/audit.c index 196d327..31cb11d 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1063,6 +1063,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature return; ab = audit_log_start(context, GFP_KERNEL, AUDIT_FEATURE_CHANGE); + if (!ab) + return; audit_log_task_info(ab, current); audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", audit_feature_names[which], !!old_feature, !!new_feature, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 6:47 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 6:47 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML Cc: Paul Moore, Eric Paris, Steve Grubb, Richard Guy Briggs If there is a memory allocation error when trying to change an audit kernel feature value, the ignored allocation error will trigger a NULL pointer dereference oops on subsequent use of that pointer. Return instead. See: https://github.com/linux-audit/audit-kernel/issues/76 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- kernel/audit.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/audit.c b/kernel/audit.c index 196d327..31cb11d 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1063,6 +1063,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature return; ab = audit_log_start(context, GFP_KERNEL, AUDIT_FEATURE_CHANGE); + if (!ab) + return; audit_log_task_info(ab, current); audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", audit_feature_names[which], !!old_feature, !!new_feature, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference 2018-02-21 6:47 ` Richard Guy Briggs @ 2018-02-21 6:52 ` Richard Guy Briggs -1 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 6:52 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML On 2018-02-21 01:47, Richard Guy Briggs wrote: > If there is a memory allocation error when trying to change an audit > kernel feature value, the ignored allocation error will trigger a NULL > pointer dereference oops on subsequent use of that pointer. Return > instead. > > See: https://github.com/linux-audit/audit-kernel/issues/76 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> Self-NACK. It was based on local development and won't compile on upstream. Fix pending. > --- > kernel/audit.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 196d327..31cb11d 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1063,6 +1063,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > return; > > ab = audit_log_start(context, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > + if (!ab) > + return; > audit_log_task_info(ab, current); > audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > audit_feature_names[which], !!old_feature, !!new_feature, > -- > 1.8.3.1 > - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] audit: return on memory error to avoid null pointer dereference @ 2018-02-21 6:52 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2018-02-21 6:52 UTC (permalink / raw) To: Linux-Audit Mailing List, LKML; +Cc: Paul Moore, Eric Paris, Steve Grubb On 2018-02-21 01:47, Richard Guy Briggs wrote: > If there is a memory allocation error when trying to change an audit > kernel feature value, the ignored allocation error will trigger a NULL > pointer dereference oops on subsequent use of that pointer. Return > instead. > > See: https://github.com/linux-audit/audit-kernel/issues/76 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> Self-NACK. It was based on local development and won't compile on upstream. Fix pending. > --- > kernel/audit.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/audit.c b/kernel/audit.c > index 196d327..31cb11d 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -1063,6 +1063,8 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature > return; > > ab = audit_log_start(context, GFP_KERNEL, AUDIT_FEATURE_CHANGE); > + if (!ab) > + return; > audit_log_task_info(ab, current); > audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d", > audit_feature_names[which], !!old_feature, !!new_feature, > -- > 1.8.3.1 > - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-02-22 0:59 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-21 9:30 [PATCH] audit: return on memory error to avoid null pointer dereference Richard Guy Briggs 2018-02-21 9:30 ` Richard Guy Briggs 2018-02-21 23:49 ` Paul Moore 2018-02-21 23:49 ` Paul Moore 2018-02-22 0:02 ` Paul Moore 2018-02-22 0:02 ` Paul Moore 2018-02-22 0:34 ` Eric Paris 2018-02-22 0:34 ` Eric Paris 2018-02-22 0:55 ` Richard Guy Briggs 2018-02-22 0:55 ` Richard Guy Briggs -- strict thread matches above, loose matches on Subject: below -- 2018-02-21 6:47 Richard Guy Briggs 2018-02-21 6:47 ` Richard Guy Briggs 2018-02-21 6:52 ` Richard Guy Briggs 2018-02-21 6:52 ` Richard Guy Briggs
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.