lkml.org 
[lkml]   [2011]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 2/3] printk: Add %ptc to safely print a task's comm
    From
    Date
    On Tue, 2011-05-10 at 17:51 -0700, Joe Perches wrote:
    > On Tue, 2011-05-10 at 17:23 -0700, John Stultz wrote:
    > > Acessing task->comm requires proper locking. However in the past
    > > access to current->comm could be done without locking. This
    > > is no longer the case, so all comm access needs to be done
    > > while holding the comm_lock.
    > >
    > > In my attempt to clean up unprotected comm access, I've noticed
    > > most comm access is done for printk output. To simpify correct
    > > locking in these cases, I've introduced a new %ptc format,
    > > which will safely print the corresponding task's comm.
    >
    > Hi John.
    >
    > Couple of tyops for Accessing and simplify in your commit message
    > and a few comments on the patch.

    Ah. Yes. Thanks!

    > Could misuse of %ptc (not using current) cause system lockup?

    It very well could. Although I don't see other %p options tring to
    handle invalid pointers. Any suggestions on how to best handle this?


    > > Example use:
    > > printk("%ptc: unaligned epc - sending SIGBUS.\n", current);
    >
    >
    > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
    > > index bc0ac6b..b9c97b8 100644
    > > --- a/lib/vsprintf.c
    > > +++ b/lib/vsprintf.c
    > > @@ -797,6 +797,26 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
    > > return string(buf, end, uuid, spec);
    > > }
    > >
    > > +static noinline_for_stack
    > > +char *task_comm_string(char *buf, char *end, u8 *addr,
    > > + struct printf_spec spec, const char *fmt)
    >
    > addr should be void * not u8 *
    >
    > > +{
    > > + struct task_struct *tsk = (struct task_struct *) addr;
    >
    > no cast.
    >
    > Maybe it'd be better to use current inside this routine and not
    > pass the pointer at all.

    That sounds reasonable. Most users are current, so forcing the more rare
    non-current users to copy it to a buffer first and use the normal %s
    would not be of much impact.

    Although I'm not sure if there's precedent for a %p value that didn't
    take a argument. Thoughts on that? Anyone else have an opinion here?

    Thanks so much for the review and feedback!
    -john



    \
     
     \ /
      Last update: 2011-05-11 03:13    [W:0.025 / U:61.260 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site