Re: [PATCH] Allow threads to rename siblings via /proc/pid/tasks/tid/comm

[Andi Kleen]

john stultz <johnstul@xxxxxxxxxx> writes:
> -	strlcpy(tsk->comm, buf, sizeof(tsk->comm));
> +
> +	/*
> +	 * Threads may access current->comm without holding
> +	 * the task lock, so write the string carefully
> +	 * to avoid non-terminating reads. Readers without a lock
> +	 * will get the oldname, the newname or an empty string.
> +	 */
> +	tsk->comm[0] = 0;
> +	wmb();
> +	strlcpy(tsk->comm+1, buf+1, sizeof(tsk->comm)-1);
> +	wmb();
> +	tsk->comm[0] = buf[0];

Is this really safe? 

reader                    writer


read comm[0]
                         set comm[0] to 0
                         overwrites comm[1]
read comm[1]
read comm[2]
                         writes comm[2] to 0 
read comm[3]

...
goes beyond the end
                  
Better way probably is to replace tsk->comm with a pointer
and exchange that using xchg. Drawback: 4-8 bytes more per task.

Or perhaps make comm one byte longer and make sure the last
byte is always 0, but the drawback is that a reader can
read random (but at least safe) junk then.

-Andi

-- 
ak@xxxxxxxxxxxxxxx -- Speaking for myself only.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/