mm: vmscan: make unregister_shrinker() safer
Subsystems (3)
PATCH 1/1 mm: vmscan: make unregister_shrinker() safer
unregister_shrinker() does not have any sanitizing inside so calling it twice will oops because of double free attempt or so. This patch makes unregister_shrinker() safer and allows calling it on resource freeing path without explicit knowledge of whether shrinker was successfully registered or not.
--- mm/vmscan.c | 4 ++++ 1 file changed, 4 insertions(+)
mm/vmscan.c +4 -0
@@ -281,10 +281,14 @@ EXPORT_SYMBOL(register_shrinker);*/ void unregister_shrinker (struct shrinker *shrinker) { +if (!shrinker->nr_deferred) +return ; +down_write (&shrinker_rwsem);list_del (&shrinker->list);up_write (&shrinker_rwsem);kfree (shrinker->nr_deferred); + shrinker->nr_deferred =NULL ; }EXPORT_SYMBOL (unregister_shrinker);
Signature · 7 lines
2.11.0 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Michal Hocko
reviewerQuoted 5 lines · Aliaksei Karaliou
unregister_shrinker()does not have any sanitizing inside so calling it twice will oops because of double free attempt or so. This patch makesunregister_shrinker()safer and allows calling it on resource freeing path without explicit knowledge of whether shrinker was successfully registered or not.
Tetsuo has made it half way to this already [1]. So maybe we should fold shrinker->nr_deferred = NULL to his patch and finally merge it.
[1] http://lkml.kernel.org/r/1511523385-6433-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
Quoted 28 lines
Signed-off-by Aliaksei Karaliou <[email protected]>--- mm/vmscan.c | 4 ++++ 1 file changed, 4 insertions(+)mm/vmscan.c +4
-0@@ -281,10 +281,14 @@ EXPORT_SYMBOL(register_shrinker);*/ void unregister_shrinker (struct shrinker *shrinker) { +if (!shrinker->nr_deferred) +return ; +down_write (&shrinker_rwsem);list_del (&shrinker->list);up_write (&shrinker_rwsem);kfree (shrinker->nr_deferred); + shrinker->nr_deferred =NULL ; }EXPORT_SYMBOL (unregister_shrinker);Signature · 1 line
2.11.0
Signature · 8 lines
Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 10 lines · Michal Hocko
Quoted 5 lines · Aliaksei Karaliou
unregister_shrinker()does not have any sanitizing inside so calling it twice will oops because of double free attempt or so. This patch makesunregister_shrinker()safer and allows calling it on resource freeing path without explicit knowledge of whether shrinker was successfully registered or not.Tetsuo has made it half way to this already [1]. So maybe we should fold
shrinker->nr_deferred= NULL to his patch and finally merge it.[1] http://lkml.kernel.org/r/1511523385-6433-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
Yeah, no problem from my side.
I'm sorry, it seems that I haven't done enough research to realize that someone is already looking at that place.
The only my concern/question is whether we should also add some paranoid stuff in that extra branch (check that list is empty for example) or not.
Quoted 24 lines
Quoted 24 lines
Signed-off-by Aliaksei Karaliou <[email protected]>--- mm/vmscan.c | 4 ++++ 1 file changed, 4 insertions(+)mm/vmscan.c +4
-0@@ -281,10 +281,14 @@ EXPORT_SYMBOL(register_shrinker);*/ void unregister_shrinker (struct shrinker *shrinker) { +if (!shrinker->nr_deferred) +return ; +down_write (&shrinker_rwsem);list_del (&shrinker->list);up_write (&shrinker_rwsem);kfree (shrinker->nr_deferred); + shrinker->nr_deferred =NULL ; }EXPORT_SYMBOL (unregister_shrinker);
Quoted 3 lines
Quoted 3 lines
Signature · 1 line
2.11.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Michal Hocko
reviewerQuoted 8 lines · ak
Quoted 6 lines · Michal Hocko
Quoted 5 lines · Aliaksei Karaliou
unregister_shrinker()does not have any sanitizing inside so calling it twice will oops because of double free attempt or so. This patch makesunregister_shrinker()safer and allows calling it on resource freeing path without explicit knowledge of whether shrinker was successfully registered or not.
Quoted 8 lines
Quoted 4 lines
Tetsuo has made it half way to this already [1]. So maybe we should fold
shrinker->nr_deferred= NULL to his patch and finally merge it.[1] http://lkml.kernel.org/r/1511523385-6433-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
Yeah, no problem from my side.
I'm sorry, it seems that I haven't done enough research to realize that someone is already looking at that place.
Absolutely no reason to be worried. This happens all the time ;)
Quoted 3 lines
The only my concern/question is whether we should also add some paranoid stuff in that extra branch (check that list is empty for example) or not.
I wouldn't bother. There are two reasons to actually care here: a) to make registration code easier (so that they can call unregister_shrinker even on path with failed register_shrinker - e.g. sget_userns would become more complex if we had to special case the failure) and b) to not blow up on the double unregister which is an alternative of a).
We really do not need this to be super clever, it is an internal function.
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Signed-off-by Aliaksei Karaliou <[email protected]>--- mm/vmscan.c | 4 ++++ 1 file changed, 4 insertions(+)mm/vmscan.c +4
-0@@ -281,10 +281,14 @@ EXPORT_SYMBOL(register_shrinker);*/ void unregister_shrinker (struct shrinker *shrinker) { +if (!shrinker->nr_deferred) +return ; +down_write (&shrinker_rwsem);list_del (&shrinker->list);up_write (&shrinker_rwsem);kfree (shrinker->nr_deferred); + shrinker->nr_deferred =NULL ; }EXPORT_SYMBOL (unregister_shrinker);Signature · 1 line
2.11.0
Signature · 8 lines
Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>