On Sat, Jan 11, 2014 at 12:59 AM, Andrew Morton <a...@linux-foundation.org> wrote: >> +char *comm_name(char *buf, char *end, struct task_struct *tsk, >> + struct printf_spec spec, const char *fmt) >> +{ >> + char name[TASK_COMM_LEN]; >> + >> + /* Caller can pass NULL instead of current. */ >> + if (!tsk) >> + tsk = current; >> + /* Not using get_task_comm() in case I'm in IRQ context. */ >> + memcpy(name, tsk->comm, TASK_COMM_LEN);
So this may copy more bytes than the actual string length of tsk->comm. As this is a temporary buffer, that just wastes cycles. And even if it wasn't, data between the string zero terminator and the end of the buffer wouild be leaked. >> + name[sizeof(name) - 1] = '\0'; You can use strlcpy() here instead of memcpy and clear. > get_task_comm() uses strncpy()? char *get_task_comm(char *buf, struct task_struct *tsk) { /* buf must be at least sizeof(tsk->comm) in size */ task_lock(tsk); strncpy(buf, tsk->comm, sizeof(tsk->comm)); task_unlock(tsk); return buf; } Is get_task_comm() used to prepare data for userspace? strncpy() fills the remaining of the buffer with zeroes, so it avoids leaking data. Note that strncpy() may leave the buffer non-zero-terminated if the source string is too long, but as set_task_comm() uses strlcpy(), this should never be the case: void set_task_comm(struct task_struct *tsk, char *buf) { task_lock(tsk); trace_task_rename(tsk, buf); strlcpy(tsk->comm, buf, sizeof(tsk->comm)); task_unlock(tsk); perf_event_comm(tsk); } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/