回复: 回复: [PATCH] mm: Fix possible NULL pointer dereference in __swap_duplicate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > >
> > > > swp_swap_info() may return null; it is necessary to check the
> > > > return value to avoid NULL pointer dereference. The code for other
> > > > calls to
> > > > swp_swap_info() includes checks, and __swap_duplicate() should
> > > > also include checks.
> > >
> > > Actually very few of the swp_swap_info() callers check for a NULL return.
> > The swapfile.c file contains three instances where the return value of
> > swp_swap_info() is checked for a NULL return. In other files that call
> > swp_swap_info(), I have confirmed that there are no such checks.
> > The description in the patch is inaccurate, and I have made
> > modifications in patch v2.
> > >
> > > > The reason why swp_swap_info() returns NULL is unclear; it may be
> > > > due to CPU cache issues or DDR bit flips.
> > >
> > > Quite possibly it's a kernel bug.
> > >
> > > > The probability of this issue is very small, and the stack info we
> > > > encountered is as follows:
> > > > Unable to handle kernel NULL pointer dereference at virtual
> > > > address
> > > > 0000000000000058
> > > >
> > > > ...
> > > >
> > > > --- a/mm/swapfile.c
> > > > +++ b/mm/swapfile.c
> > > > @@ -3521,6 +3521,8 @@ static int __swap_duplicate(swp_entry_t
> > > > entry,
> > > unsigned char usage, int nr)
> > > >  	int err, i;
> > > >
> > > >  	si = swp_swap_info(entry);
> > > > +	if (unlikely(!si))
> > > > +		return -EINVAL;
> > > >
> > > >  	offset = swp_offset(entry);
> > > >  	VM_WARN_ON(nr > SWAPFILE_CLUSTER - offset %
> SWAPFILE_CLUSTER);
> > >
> > > OK, I guess avoiding the crash is good.  But please let's include a
> > > WARN so that we can perhaps fix the bug, if one is there.
> > Good. I'll change it as mentioned and send a new patch.
> > 	si = swp_swap_info(entry);
> > +	if (unlikely(!si)) {
> > +		WARN(1, KERN_ERR "%s: %s%08lx\n", __func__, Bad_file, entry.val);
> 
> WARN() already contains unlikely(). Also, no need to print the function name it's
> already in the stack trace.
> 
> We should probably just do if (WARN_ON_ONCE(!si)).
> 
> > +		return -EINVAL;
> > +	}
Yes, thank you for your suggestion. This modification makes the code simpler.
	si = swp_swap_info(entry);
+	if (WARN_ON_ONCE(!si))
+		return -EINVAL;

> >
> >




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux