Bug #8978

Fiddle possibly misuses mprotect

Added by Yusuke Endoh 7 months ago. Updated about 1 month ago.

[ruby-core:57599]
Status:Closed
Priority:Normal
Assignee:Aaron Patterson
Category:ext
Target version:2.1.0
ruby -v:ruby 2.1.0dev (2013-10-02 trunk 43121) [x86_64-linux] Backport:1.9.3: DONE, 2.0.0: DONE, 2.1: DONE

Description

Hello Aaron,

Coverity Scan found a possible bug in "initialize" function of ext/fiddle/closure.c:

result = ffiprepclosure(pcl, cif, callback, (void *)self);
...
i = mprotect(pcl, sizeof(pcl), PROTREAD | PROTEXEC)

I don't understand the code completely, but the size of the pointer does not seem to make sense.
Perhaps, "sizeof(pcl)" should be "sizeof(*pcl)".

The same applies to dealloc:

munmap(cls->pc1, sizeof(cls->pc1));

BTW, ffiprepclosure seems deprecated.
We should use ffiprepclosure_loc instead when it is available.

Yusuke Endoh mame@tsg.ne.jp

Associated revisions

Revision 44731
Added by tenderlove 3 months ago

  • ext/fiddle/closure.c: use sizeof(*pcl) for correct sizeof value. [Bug #8978]. Thanks mame!

Revision 44751
Added by Yusuke Endoh 3 months ago

  • ext/fiddle/closure.c: use sizeof(*pcl) for correct sizeof value. [Bug #8978].

History

#1 Updated by Anonymous 3 months ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

Applied in changeset r44731.


  • ext/fiddle/closure.c: use sizeof(*pcl) for correct sizeof value. [Bug #8978]. Thanks mame!

#2 Updated by Aaron Patterson 3 months ago

I took the mprotect example from the ffi man pages. Seems there must be a bug in the example code. Anyway, I've fixed it.

Also, we should be using ffi_prep_closure_loc if it is available:

https://github.com/ruby/ruby/blob/92a7f323678d6070383364f2129a5202b217c5da/ext/fiddle/closure.c#L231-234

#3 Updated by Tomoyuki Chikanaga 3 months ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED

#4 Updated by Yusuke Endoh 3 months ago

Thank you!

Nagachika, I think r44751 is also needed to backport.

Yusuke Endoh mame@tsg.ne.jp

#5 Updated by Usaku NAKAMURA 2 months ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: DONE, 2.0.0: REQUIRED, 2.1: REQUIRED

backported into ruby19_3 at r44941.

#6 Updated by Tomoyuki Chikanaga 2 months ago

  • Backport changed from 1.9.3: DONE, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: DONE, 2.0.0: DONE, 2.1: REQUIRED

r44731 and r44751 were backported to ruby20_0 at r45008.

#7 Updated by Yui NARUSE about 1 month ago

  • Backport changed from 1.9.3: DONE, 2.0.0: DONE, 2.1: REQUIRED to 1.9.3: DONE, 2.0.0: DONE, 2.1: DONE

r45122

Also available in: Atom PDF