lkml.org 
[lkml]   [2012]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] remove no longer use of pdflush interface
    On Tue, Jun 05, 2012 at 03:28:45PM -0700, Andrew Morton wrote:


    Hi Andrew,
    >I do agree with the intent of the patch and the creation of
    >proc_deprecated() seems a good idea - something we can use in the
    >future.

    Yes, actually I think many interfaces in /proc which will be removed in
    the near future can take advantage of this function.

    >
    >> --- a/kernel/sysctl.c
    >> +++ b/kernel/sysctl.c
    >> @@ -1095,11 +1095,9 @@ static struct ctl_table vm_table[] = {
    >> .extra1 = &zero,
    >> },
    >> {
    >> - .procname = "nr_pdflush_threads",
    >> - .data = &nr_pdflush_threads,
    >> - .maxlen = sizeof nr_pdflush_threads,
    >> - .mode = 0444 /* read-only*/,
    >> - .proc_handler = proc_dointvec,
    >> + .procname = "nr_pdflush_threads",
    >> + .mode = 0444 /* read-only */,
    >> + .proc_handler = proc_deprecated,
    >> },
    >> {
    >> .procname = "swappiness",
    >> @@ -2505,6 +2503,15 @@ int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int write,
    >>
    >> #endif /* CONFIG_PROC_SYSCTL */
    >>
    >> +/* notice associated proc deprecated */
    >> +int proc_deprecated(struct ctl_table *table, int write,
    >> + void __user *buffer, size_t *lenp, loff_t *ppos)
    >> +{
    >> + printk(KERN_WARNING "%s exported in /proc is deprecated\n",
    >> + table->procname);
    >> + return -ENOSYS;
    >> +}
    >
    >I see a couple of things here.
    >
    >Firstly, I'd change the text from "deprecated" to "is scheduled for
    >removal". Which implies that the function should be called
    >proc_obsolete().
    >
    >
    >Secondly, this code will permit unprivileged users to flood the logs,
    >by repeatedly reading /proc/sys/vm/nr_pdflush_threads. We try to avoid
    >this, as it is a form of denial-of-service attack.
    >
    >This is a bit hard to fix. The typical way of addressing this is to
    >use printk_once(), so the message only appears once per boot. But that
    >doesn't work for a generic function - we'd need to add one bit of state
    >to the ctl_table to do this. We can of course do that, but it's not
    >obvious that it's _worth_ doing that just for handling obsolete
    >entries.
    >
    >So perhaps the solution is to give up on the generic proc_obsolete()
    >idea, and just add a handler specifically for nr_pdflush_threads, whcih
    >uses printk_once().

    What about modify the generic proc_obsolete just to put the warning message into
    the buffer, then transfer to userspace, in order to users can see this warning.Do
    you think this is a better idea?

    Regards,
    Wanpeng



    \
     
     \ /
      Last update: 2012-06-06 05:41    [W:0.027 / U:0.212 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site