From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Morris Subject: Re: [PATCH] lsm: copy comm before calling audit_log to avoid race in string printing Date: Wed, 15 Apr 2015 09:55:24 +1000 (AEST) Message-ID: References: <2762230afd590c4c6f50c4905ef91738a4f38e4c.1429023587.git.rgb@redhat.com> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Return-path: In-Reply-To: <2762230afd590c4c6f50c4905ef91738a4f38e4c.1429023587.git.rgb@redhat.com> Sender: linux-security-module-owner@vger.kernel.org To: Richard Guy Briggs Cc: linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-audit@redhat.com, sgrubb@redhat.com, eparis@parisplace.org, pmoore@redhat.com, penguin-kernel@I-love.SAKURA.ne.jp List-Id: linux-audit@redhat.com On Tue, 14 Apr 2015, Richard Guy Briggs wrote: > When task->comm is passed directly to audit_log_untrustedstring() without > getting a copy or using the task_lock, there is a race that could happen that > would output a NULL (\0) in the middle of the output string that would > effectively truncate the rest of the report text after the comm= field in the > audit log message, losing fields. > > Using get_task_comm() to get a copy while acquiring the task_lock to prevent > this and to prevent the result from being a mixture of old and new values of > comm would incur potentially unacceptable overhead, considering that the value > can be influenced by userspace and therefore untrusted anyways. > > Copy the value before passing it to audit_log_untrustedstring() ensures that a > local copy is used to calculate the length *and* subsequently printed. Even if > this value contains a mix of old and new values, it will only calculate and > copy up to the first NULL, preventing the rest of the audit log message being > truncated. > > Use a second local copy of comm to avoid a race between the first and second > calls to audit_log_untrustedstring() with comm. > > Reported-by: Tetsuo Handa > Signed-off-by: Richard Guy Briggs Applied. -- James Morris