Bug 10731 - sys_poll_intr(): wrong unit for timeout value
Summary: sys_poll_intr(): wrong unit for timeout value
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.1.9
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-21 10:04 UTC by Daniel Kobras (dead mail address)
Modified: 2014-09-01 19:16 UTC (History)
1 user (show)

See Also:


Attachments
sys_poll_intr: fix timeout arithmetic (1.18 KB, patch)
2014-07-21 10:04 UTC, Daniel Kobras (dead mail address)
no flags Details
Patch in git-format patch format for master (1.34 KB, patch)
2014-07-21 13:59 UTC, Volker Lendecke
metze: review+
Details
Patch for v4-1 (1.61 KB, patch)
2014-07-22 08:27 UTC, Volker Lendecke
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Kobras (dead mail address) 2014-07-21 10:04:55 UTC
Created attachment 10130 [details]
sys_poll_intr: fix timeout arithmetic

Callers of sys_poll_intr() assume the timeout parameter to be in milliseconds, just like poll(2) expects it. If the poll() call gets interrupted before the timeout expires, the timeout arithmetic in sys_poll_intr() assumes nanosecond units instead. This will usually lead to a negative new timeout value. Hence, after an EINTR, sys_poll_intr() might hang.

I'm attaching a patch that should fix this problem in sys_poll_intr().

Both patches were created against 4.1.9, but should apply to master as well.

(We only noticed the bug while calling receive_smb_talloc() with a non-zero timeout for debugging purposes. In the unmodified code, few callers seem to end up in sys_poll_intr() with a positive timeout.)
Comment 1 Volker Lendecke 2014-07-21 13:59:22 UTC
Created attachment 10133 [details]
Patch in git-format patch format for master

Entirely correct, thanks Daniel!

For future patches, it would be great if you could use "git format-patch --stdout" to upload patches. This makes it easier to apply them locally with "git am -3".

Volker
Comment 2 Stefan Metzmacher 2014-07-21 15:22:20 UTC
(In reply to comment #1)
> Created attachment 10133 [details]
> Patch in git-format patch format
> 
> Entirely correct, thanks Daniel!
> 
> For future patches, it would be great if you could use "git format-patch
> --stdout" to upload patches. This makes it easier to apply them locally with
> "git am -3".
> 
> Volker

Looks ok.
Comment 3 Volker Lendecke 2014-07-22 08:27:20 UTC
Created attachment 10142 [details]
Patch for v4-1

This is what ended up in master, with all required tags
Comment 4 Stefan Metzmacher 2014-07-22 15:08:25 UTC
What about v4-0*?
Comment 5 Volker Lendecke 2014-07-22 15:53:34 UTC
(In reply to comment #4)
> What about v4-0*?

Well, the code is the same there, thus it also has the same bug. The patch applies cleanly to v4-0-test as well. I'll leave it to Karolin whether it's worth also taking for 4.0.
Comment 6 Karolin Seeger 2014-08-03 18:36:41 UTC
Pushed to autobuild-v4-[0|1]-test.
Comment 7 Karolin Seeger 2014-09-01 19:16:57 UTC
(In reply to comment #6)
> Pushed to autobuild-v4-[0|1]-test.

Pushed to both branches.
Closing out bug report.

Thanks!