On Tue 07-05-13 23:29:58, Jeff Liu wrote: > On 05/07/2013 11:15 PM, Michal Hocko wrote: > > On Tue 07-05-13 23:04:54, Jeff Liu wrote: > >> On 05/07/2013 10:12 PM, Michal Hocko wrote: > >>> On Sun 05-05-13 23:44:41, Sha Zhengju wrote: > >>>> memparse() doesn't check if overflow has happens, and it even has no > >>>> args to inform user that the unexpected situation has occurred. Besides, > >>>> some of its callers make a little artful use of the current implementation > >>>> and it also seems to involve too much if changing memparse() interface. > >>>> > >>>> This patch rewrites memcg's internal res_counter_memparse_write_strategy(). > >>>> It doesn't use memparse() any more and replaces simple_strtoull() with > >>>> kstrtoull() to avoid input overflow. > >>> > >>> I do not like this to be honest. I do not think we should be really > >>> worried about overflows here. Or where this turned out to be a real > >>> issue? > >> Yes. e.g. > >> Without this validation, user could specify a big value larger than ULLONG_MAX > >> which would result in 0 because of an overflow. Even worse, all the processes > >> belonging to this group will be killed by OOM-Killer in this situation. > > > > I would consider this to be a configuration problem. > It mostly should be a problem of configuration. > > > >>> The new implementation is inherently slower without a good > >>> reason. > >> In talking about this, I also concerned for the overhead as per an offline > >> discussion with Sha when she wrote this fix. However, can we consider it to be > >> a tradeoff as this helper is not being used in any hot path? > > > > what is the positive part of the trade off? Fixing a potential overflow > > when somebody sets a limit to an unreasonable value? > I suppose it to be a defense for unreasonable value because this issue > is found on a production environment for an incorrect manipulation, but > it's up to you. I _really_ do not want to punish everybody just because of somthing that is a configuration issue. > > Thanks, > -Jeff -- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>