From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Hutchings Subject: Re: Report Double Fetch Bug Found in Linux-4.6.1/kernel/auditsc.c Date: Tue, 21 Jun 2016 20:59:37 +0100 Message-ID: <1466539177.27155.204.camel@decadent.org.uk> References: <20160620182215.GC25615@madcap2.tricolour.ca> <20160620191814.GA2942@redhat.com> <480FAE99-E4E1-42D0-ABD5-8DC24A7EC9BB@gmail.com> <1466502671.27155.185.camel@decadent.org.uk> <20160621181431.GD25615@madcap2.tricolour.ca> <1466533233.27155.193.camel@decadent.org.uk> <20160621191847.GB29695@madcap2.tricolour.ca> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4411919712047105496==" Return-path: In-Reply-To: <20160621191847.GB29695@madcap2.tricolour.ca> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Richard Guy Briggs Cc: security@kernel.org, Pengfei Wang , "Krinke, Jens" , Oleg Nesterov , linux-audit@redhat.com List-Id: linux-audit@redhat.com --===============4411919712047105496== Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-NGmqO5x+cCeuzY23A+F8" --=-NGmqO5x+cCeuzY23A+F8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2016-06-21 at 15:18 -0400, Richard Guy Briggs wrote: > On 2016-06-21 19:20, Ben Hutchings wrote: > > On Tue, 2016-06-21 at 14:14 -0400, Richard Guy Briggs wrote: > > > On 2016-06-21 10:51, Ben Hutchings wrote: > > > > On Tue, 2016-06-21 at 10:37 +0100, Pengfei Wang wrote: > > > > > >=20 > > > > > > =E5=9C=A8 2016=E5=B9=B46=E6=9C=8820=E6=97=A5=EF=BC=8C=E4=B8=8B= =E5=8D=888:18=EF=BC=8COleg Nesterov =E5=86=99=E9=81=93=EF= =BC=9A > > > > > >=20 > > > > > > Not that I understand this report, but > > > > > >=20 > > > > > > On 06/20, Richard Guy Briggs wrote: > > > > > > >=20 > > > > > > > This function is only ever called by __audit_free(), which is= only ever > > > > > > > called on failure of task creation or on exit of the task, so= in neither > > > > > > > case can anything else change it. > > > > > >=20 > > > > > > How so? > > > > > >=20 > > > > > > Another thread or CLONE_VM task or /proc/pid/mem can change the= user-space > > > > > > memory in parallel. > > > > > >=20 > > > > > > Oleg. > > > > >=20 > > > > >=20 > > > > > Exactly, by saying =E2=80=9Cchange the data=E2=80=9D, I mean the = modification from > > > > > malicious users with crafted operations on the user space memory > > > > > directly, rather than the normal operations within the audit > > > > > subsystem in Linux. Moreover, since the copy operations from the = user > > > > > space are not protected by any locks or synchronization primitive= s, > > > > > changing the data under race condition is feasible I think. Besid= es, > > > > > there isn=E2=80=99t any visible checking step in the code to guar= antee the > > > > > consistency between the two copy operations. > > > > >=20 > > > > > Here I would like=C2=A0to figure out what the consequences really= are once > > > > > the data is changed between the two copy operations, such as chan= ging > > > > > a none-control string to a control string but process it as a non= e- > > > > > control string that has no control chars. I think problems will > > > > > happen. > > > >=20 > > > > So far as userland can see, kernel log lines are separated by newli= nes. > > >=20 > > > Newlines are control characters that would be caught by that filter. > > > That filter catches '"', < 0x21, > 0x7e. > > >=20 > > > > If we fail to escape a newline, that makes it possible to inject > > > > arbitrary log lines into the kernel log, which may be misleading to= the > > > > administrator or to software parsing the log. > > >=20 > > > So, this is addressed, but I'm still trying to assess the danger of t= his > > > repeated call to copy_from_user(). > >=20 > > The problem is that newlines can be added to the strings by another > > task between the first pass that checks for control characters and the > > second pass that copies them to the log. >=20 > Understood, so this is the same sort of problem as Pengfei has raised > with respect to double quotes being added. >=20 > How can subsequent accesses of copy_from_user() be locked, or make sure > the entire buffer is copied in one go? I don't believe it can. =C2=A0And the fact that those strings can be modified before they're logged kind of defeats the purpose of auditing, no? =C2=A0Seems like it would make more sense to copy the program name from the binprm, log that at this point and don't even attempt to log the arguments. Ben. --=20 Ben Hutchings [W]e found...that it wasn't as easy to get programs right as we had thought. ... I realized that a large part of my life from then on was going to be spent in finding mistakes in my own programs. - Maurice Wilkes, 1949 --=-NGmqO5x+cCeuzY23A+F8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCgAGBQJXaZypAAoJEOe/yOyVhhEJNQ8P/ApcwEXOzBvUHIExbtGD5yaX QFpJc02k5e6wW3as6FTxq1Zebrb46NSFKaa0kdSbhsnyiHTkaxHgW4rrWnQjqNau +w3i6QqwoKeMRha+Yzd188axr/ezN2TpGVsqZR8d+fukIPIfv86x9xeWlFyK5uIz iAn99WeJEbA/BmaNgc5Um/qkRu2eMSabIsIg41yJIZrUFfFb0G9RTRKdj8mJsxd1 XBSWP9e6CCT3wFx3KvB9RLM0LwhubGIiwQLchJ0q6fgPGMhK9y2yiESZsFC8fdQ0 hiAwDemF+B4M/ohJco/V6NUFS0ZjE7WoQeUOFNIVyFsebYDM9TkrlajWgnR/lI58 KDGsTTus9lzljCOvjGgmz1waT5x1XVIl/PUJ+lYwWo7LAT9APWVg7nnujZyKKKNh da0AcmEZGhHTOLTnNn1+1l98toDM4ZDKYafmL4SsT/1Kj3XBbblCs6vA54i3d1GO W2fipTYZlzesQtF9Zrk6/gsNTArmBobCB7XhYH4OHQyS1u+30IMMcGpL22EK0d00 QKOU/l2Tfs71BwUHKgJK8tLFK44UW4G8hpKtX79HEOE4OgGnRZjyT4x/54tdXQlo +plNJ1yD7S1WlH3bl4pwt6EhZStaQBH2A9GDGG5hsnF2kKaioItGUEEeozTFVvIW hrIIkQHks5Jp01tMZ0dJ =OJE5 -----END PGP SIGNATURE----- --=-NGmqO5x+cCeuzY23A+F8-- --===============4411919712047105496== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============4411919712047105496==--