Bug 1034894 - memstomp: does not intercept fortify chk versions
Summary: memstomp: does not intercept fortify chk versions
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Developer Toolset
Classification: Red Hat
Component: memstomp
Version: DTS 2.0 RHEL 6
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: alpha
: 5.0
Assignee: Jeff Law
QA Contact: Martin Cermak
URL:
Whiteboard:
Depends On:
Blocks: 1034888
TreeView+ depends on / blocked
 
Reported: 2013-11-26 16:41 UTC by Florian Weimer
Modified: 2019-07-16 15:22 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1175280 (view as bug list)
Environment:
Last Closed: 2019-07-16 15:22:37 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Florian Weimer 2013-11-26 16:41:31 UTC
The checking functions (for example, __memcpy_chk, __wmemcpy_chk) should be intercepted as well.  The s*printf family of functions is missing as well.

Comment 2 Jeff Law 2013-11-26 21:11:05 UTC
Code contributions always welcome :-)  I suspect the printf family would be fairly painful to handle.

My understanding of the _chk thingies is that when not expanded inline by the compiler, they will call their appropriate glibc function.  ie, memcpy_chk will call memcpy if the compiler doesn't expand the memcpy_chk inline.  So there's nothing to do for them.  Is that not the case?

Note memstomp simply can't catch things which are inline expanded by the compiler.  No way around that.

Comment 3 Florian Weimer 2013-11-26 21:41:10 UTC
(In reply to Jeff Law from comment #2)

> My understanding of the _chk thingies is that when not expanded inline by
> the compiler, they will call their appropriate glibc function.  ie,
> memcpy_chk will call memcpy if the compiler doesn't expand the memcpy_chk
> inline.  So there's nothing to do for them.  Is that not the case?

Surprisingly, no.  __memcpy_chk has an open-coded version.  If we change that, we risk uncovering further overlapping buffers, but perhaps we should make the change upstream, so that we can eventually benefit from all those memcpy optimizations.

> Note memstomp simply can't catch things which are inline expanded by the
> compiler.  No way around that.

Well, there is -ftree-loop-distribute-patterns, which could reverse that (sort of). :)

Covering sprintf probably isn't worth the trouble.  Maybe that's something where glibc could help.

Comment 4 Jeff Law 2013-12-09 22:59:31 UTC
Egad.  There's a wonderful set of _chk entry points.  There's a nice collection of mem* and str* functions and the wide variants of the str* functions. 


I also just realized that even if the _chk function calls the real one after passing the length check, it's probably done in a way that wouldn't allow memstomp to intercept those calls/jumps.  What a mess.

Upstream for memstomp is Will Cohen (and John Reiser).  I don't think any significant changes have been made in a couple years.  We should probably consider Fedora's repository the effective upstream repo and try to get this into F21 & RHEL 7.1.

Comment 9 Jeff Law 2014-07-22 21:51:47 UTC
No work has been done on this issue; there's effectively no chance it's going to show up in in this release of DTS.  Deferring for next release.

As we've discussed before the "rebase" brought in no new upstream code, just a tag.  There is a patch in rawhide to enable checking of certain str* and mem* for NULL pointers in places where a NULL pointer is invalid.  Pulling in those changes is a completely separate issue.

Comment 11 Jeff Law 2014-07-24 21:52:19 UTC
My gut tells me to wait on the NULL checking.

While I've got some code to do that upstream, I haven't added testsuites for that code yet and the new code hasn't had all that much testing.  It may be something we can add for 3.1 since it's a single, isolated patch.

It would also be good to do another once-over for functions we want to check for invalid NULL arguments.  Basically there's so many functions where NULL pointers aren't allowed that covering them all would be insane.  There's a *much* smaller set where the checking is actually useful.  Basically we want to check where those NULL pointers may _not_ be dereferenced by the runtime due to other arguments (such as a length).  Those are the cases where we want to add the interposition routines for checking purposes.

Comment 18 Jeff Law 2019-07-16 15:22:37 UTC
Realistically memstomp never turned out to be all that useful.  Investing further here isn't on the radar.


Note You need to log in before you can comment on or make changes to this bug.