From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751487AbcFXPU4 (ORCPT ); Fri, 24 Jun 2016 11:20:56 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:6569 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751121AbcFXPUz (ORCPT ); Fri, 24 Jun 2016 11:20:55 -0400 X-IBM-Helo: d06dlp02.portsmouth.uk.ibm.com X-IBM-MailFrom: heiko.carstens@de.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org;linux-next@vger.kernel.org Date: Fri, 24 Jun 2016 17:20:46 +0200 From: Heiko Carstens To: Paul Moore Cc: Stephen Rothwell , James Morris , linux-next@vger.kernel.org, linux-kernel@vger.kernel.org, Kees Cook , Martin Schwidefsky Subject: Re: linux-next: manual merge of the audit tree with the security tree References: <20160623141814.5512ffd1@canb.auug.org.au> <20160623060113.GA3866@osiris> <20160624054131.GA3940@osiris> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16062415-0016-0000-0000-000001FF510E X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16062415-0017-0000-0000-0000220B28B8 Message-Id: <20160624152046.GB3940@osiris> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-06-24_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1606240166 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 24, 2016 at 11:05:33AM -0400, Paul Moore wrote: > >> >> + audit_syscall_entry(regs->gprs[2], regs->orig_gpr2 & mask, > >> >> + regs->gprs[3] & mask, regs->gprs[4] & mask, > >> >> + regs->gprs[5] & mask); > >> > > >> > With these masks it is more correct, however these are still not the values > >> > used by the system call itself. This would be still incorrect for > >> > e.g. compat pointers (31 bit on s390). > >> > > >> > So it seems like audit_syscall_entry should be called after all sign, zero > >> > and masking has been done? > >> > >> For someone not familiar with s390, compat or not, where would you > >> suggest we place the audit_syscall_entry() call? > > > > I was thinking of a more generic solution for all architectures: for > > example setting a new TIF flag within do_syscall_trace_enter which > > indicates that audit_syscall_entry needs be called and then add a > > conditional call to the SYSCALL_DEFINE and COMPAT_SYSCALL_DEFINE macros. > > > > That way audit_syscall_entry would always receive already properly sign and > > zero extended system call parameters. At the downside this would increase > > the kernel text size by probably ~370 conditional branches and add two more > > instructions on the system call hot path. > > > > But that's something that could be done independently from your patch, > > which already improves the current situation. > > My immediate concern is making sure that we are at least recording the > arguments correctly in the audit record. My simple tests look okay, > but as I said before, I'm far from a s390 expert and your initial > comment made it sound like there were still problems with how we were > recording the arguments. Can you either confirm that we are logging > the arguments correctly, or provide a suggestion on how to get the > right values? That would be most helpful at this point. The arguments are correct, except that they are missing sign and zero extension to full 64 bit. However I would expect that the audit subsystem will only work on the lower 32 bits anyway for compat tasks. So that shouldn't be a problem. I'm a bit concerned about user space pointers passed as argument for compat tasks. These need to mask out 33 instead of 32 bits. This is of course system call specific and I don't know enough about audit to tell if it could be a problem.