Hi Michal
The null pointer is possible when calling the dump_header, this bug was detected by LKP. Below is the context 3 months ago.
On Mon 30-07-18 19:05:50, David Rientjes wrote:
> On Mon, 30 Jul 2018, Michal Hocko wrote:
>
> > On Mon 30-07-18 17:03:20, kernel test robot wrote:
> > [...]
> > > [ 9.034310] BUG: KASAN: null-ptr-deref in dump_header+0x10c/0x448
> >
> > Could you faddr2line on the offset please?
> >
>
> It's possible that p is NULL when calling dump_header(). In this case we
> do not want to print any line concerning a victim because no oom kill has
> occurred.
> You are right. I have missed those.
> This code shouldn't be part of dump_header(), which is called from
> multiple contexts even when an oom kill has not occurred, and is
> ratelimited. The single line output should be the canonical way that
> userspace parses the log for oom victims, we can't ratelimit it.
>
> The following would be a fix patch, but it will be broken if the cgroup
> aware oom killer is removed from -mm so that the oom_group stuff can be
> merged.
> cgroup aware oom killer is going to be replaced by a new implementation
> IIUC so the fix should be based on the yuzhoujian patch. Ideally to be
> resubmitted.
> I would just suggest adding it into a function
> dump_oom_summary(struct oom_control *oc, struct task_struct *p)
> yuzhoujian could you take care of that please?
> On Mon, 30 Jul 2018, Michal Hocko wrote:
>
> > On Mon 30-07-18 17:03:20, kernel test robot wrote:
> > [...]
> > > [ 9.034310] BUG: KASAN: null-ptr-deref in dump_header+0x10c/0x448
> >
> > Could you faddr2line on the offset please?
> >
>
> It's possible that p is NULL when calling dump_header(). In this case we
> do not want to print any line concerning a victim because no oom kill has
> occurred.
> You are right. I have missed those.
> This code shouldn't be part of dump_header(), which is called from
> multiple contexts even when an oom kill has not occurred, and is
> ratelimited. The single line output should be the canonical way that
> userspace parses the log for oom victims, we can't ratelimit it.
>
> The following would be a fix patch, but it will be broken if the cgroup
> aware oom killer is removed from -mm so that the oom_group stuff can be
> merged.
> cgroup aware oom killer is going to be replaced by a new implementation
> IIUC so the fix should be based on the yuzhoujian patch. Ideally to be
> resubmitted.
> I would just suggest adding it into a function
> dump_oom_summary(struct oom_control *oc, struct task_struct *p)
> yuzhoujian could you take care of that please?
I followed David's tip and call the new func dump_oom_summary in the oom_kill_process.
> It's possible that p is NULL when calling dump_header(). In this case we
> do not want to print any line concerning a victim because no oom kill has
>occurred.
> This code shouldn't be part of dump_header(), which is called from
> multiple contexts even when an oom kill has not occurred, and is
> ratelimited. The single line output should be the canonical way that
> userspace parses the log for oom victims, we can't ratelimit it.
> The following would be a fix patch, but it will be broken if the cgroup
> aware oom killer is removed from -mm so that the oom_group stuff can be
> merged.
> do not want to print any line concerning a victim because no oom kill has
>occurred.
> This code shouldn't be part of dump_header(), which is called from
> multiple contexts even when an oom kill has not occurred, and is
> ratelimited. The single line output should be the canonical way that
> userspace parses the log for oom victims, we can't ratelimit it.
> The following would be a fix patch, but it will be broken if the cgroup
> aware oom killer is removed from -mm so that the oom_group stuff can be
> merged.
