Closed Bug 1053277 Opened 10 years ago Closed 7 years ago

Add log of invalid fd close

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: sotaro, Unassigned)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1038461 +++ Close invalid fd close could close valid fd if fd is already reused. It could cause problems like Bug 1038461 and Bug 1047149. It seems better to logout when fd close fails.
attachment 8472044 [details] [diff] [review] in Bug 1038461 did it by modifying bionic. It requests to modify bionic and is not a ideal way to do. It seems better that close() is intercepted by gecko like malloc().
mwu, how do you think about what is a good way to add "invalid fd close log"?
(In reply to Sotaro Ikeda [:sotaro] from comment #1) > attachment 8472044 [details] [diff] [review] in Bug 1038461 did it by > modifying bionic. It requests to modify bionic and is not a ideal way to do. > It seems better that close() is intercepted by gecko like malloc(). But libc is sometimes used as static lib. If want to check this case, modification to bionic seems unavoidable.
Flags: needinfo?(mwu)
We can put something in mozglue for debug builds to override the default close() implementation. Then, when one attempts to close an invalid FD, assert.
Flags: needinfo?(mwu)
Attached file dhcpd log (deleted) —
outside of b2g related process, there are some process that try to close invaild fds. It seems that they are intentional. On example is dhcpd.
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
:milan, I am clearing the blocking flag here as this is just a logging bug, feel free to seek approval if this needs to be fixed on 2.1 or NI me if I am missing info for making a blocking call here.
blocking-b2g: 2.1? → ---
I'm worried that it isn't going to even get looked at if it isn't 2.1+. And if it doesn't get resolved, we run a chance of hitting the FD related bugs, like we have in the past, and those are difficult to find (take hours of testing) and easy to assign to the wrong area, because they cause a crash far away from the culprit. So, non-2.1+, assigned to FirefoxOS->General having likely a zero chance of getting traction is why I nominated this in the first place, but if you can make it happen I don't care if it's 2.1+ :)
Flags: needinfo?(bbajaj)
(In reply to Milan Sreckovic [:milan] from comment #8) > I'm worried that it isn't going to even get looked at if it isn't 2.1+. And > if it doesn't get resolved, we run a chance of hitting the FD related bugs, > like we have in the past, and those are difficult to find (take hours of > testing) and easy to assign to the wrong area, because they cause a crash > far away from the culprit. So, non-2.1+, assigned to FirefoxOS->General > having likely a zero chance of getting traction is why I nominated this in > the first place, but if you can make it happen I don't care if it's 2.1+ :) starting with :mwu here, to see if he can help with comment #4?
Flags: needinfo?(bbajaj) → needinfo?(mwu)
I can help someone figure out how to do this, if we have someone free.
Flags: needinfo?(mwu)
I already created some temporary ptached to investigate Bug 1057220. [1] is simple and only affect to gecko. Though it could not check the invalid fd close happen out of gecko like Bug 1058366. -[1] add log of close() called only from gecko + attachment 8477861 [details] [diff] [review] in Bug 1057220 -[2] add log of close() to bionic on gonk KK + part 1 KK bionic: attachment 8477910 [details] [diff] [review] in Bug 1057220 + part 2 gecko: attachment 8477911 [details] [diff] [review] in Bug 1057220 -[3] add log of close() to bionic on gonk JB + part 1 JB bionic: attachment 8478471 [details] [diff] [review] in Bug 1054929 + part 2 gecko: attachment 8478473 [details] [diff] [review] in Bug 1054929
The approach in [1] is what I'm looking for, though you might want to use __real_close to call close rather than using syscall. Not sure if that works on B2G though - the only option there might be to directly load the symbol. Also should only be enabled on debug builds. This will actually work on libraries outside of gecko as long as the library is not bionic. We use LD_PRELOAD to load this code which forces all code to use the version of close.
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: