Messages in this thread | | | Subject | Re: [PATCH 3/4] Intel PTI implementaiton of MIPI 1149.7. | From | J Freyensee <> | Date | Fri, 22 Apr 2011 10:57:14 -0700 |
| |
On Tue, 2011-04-19 at 18:25 -0700, David Rientjes wrote: > On Tue, 19 Apr 2011, james_p_freyensee@linux.intel.com wrote: > > > +static void pti_control_frame_built_and_sent(struct pti_masterchannel *mc) > > +{ > > + struct pti_masterchannel mccontrol = {.master = CONTROL_ID, > > + .channel = 0}; > > + const char *control_format = "%3d %3d %s"; > > + > > + char comm[sizeof(current->comm) + 1]; > > + u8 control_frame[CONTROL_FRAME_LEN]; > > + > > + if (!in_interrupt()) > > + get_task_comm(comm, current); > > + else > > + strcpy(comm, "Interrupt"); > > + > > + /* Ensure our buffer is zero terminated */ > > + comm[sizeof(current->comm)] = 0; > > + > > You definitely need to use get_task_comm() here, but that means you can't > allocate char comm[] on the stack with anything but TASK_COMM_LEN, which > is small enough that it shouldn't be an issue. Otherwise there's nothing > protecting sizeof(current->comm) from changing without holding > task_lock(current).
I'm going to look at utilizing get_task_comm() more in this function, but I think I am okay even if I miss one, as I am just doing a read from it. What is written in set_task_comm() states that threads may read from current->comm without holding the task_lock(). The name could be incomplete, which would be non-ideal (but acceptable), but it's supposed to be safe from non-terminating string reads.
And it seems like the fix for
> + comm[sizeof(current->comm)] = 0;
can just be comm[TASK_COMM_LEN].
| |