uninit mem read w/signals

[d21744ff-f396-4c71-955e-7dbd2e886779]

BPO	478001
Nosy	@tim-one, @warsaw
Files	py-umr: patch to correct umr from signal()/sigaction()

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['interpreter-core'] title = 'uninit mem read w/signals' updated_at = user = 'https://bugs.python.org/nnorwitz' ``` bugs.python.org fields: ```python activity = actor = 'tim.peters' assignee = 'none' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'nnorwitz' dependencies = [] files = ['178'] hgrepos = [] issue_num = 478001 keywords = [] message_count = 5.0 messages = ['7348', '7349', '7350', '7351', '7352'] nosy_count = 3.0 nosy_names = ['tim.peters', 'barry', 'nnorwitz'] pr_nums = [] priority = 'normal' resolution = 'rejected' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue478001' versions = ['Python 2.2'] ```

[d21744ff-f396-4c71-955e-7dbd2e886779]

if the signal()/sigaction() fails, uninitialized memory is returned.

the attached patch fixes the problem (didn't test signal() path, only the sigaction() path)

Neal

[tim-one]

Logged In: YES user_id=31435

Assigned to Barry. Neal's surely correct that we shouldn't be ignoring errors, but returning NULL isn't right either.
Should we force return of SIG_ERR then? That's the only *natural* "error return" value, and there's a backward compatibility problem here since we don't document anything about error returns for the {get,set}sig functions, nor do any of the places we call these check for an error return.

[warsaw]

Logged In: YES user_id=12800

Note sigaction() returns 0 on success, -1 on failure, setting the errno code, while signal() returns SIGERR on failure. Because PyOS*sig() attempts to provide a unified interface to the two different functions, we can't support both, and forcing a return of SIG_ERR when sigaction() fails seems to make the most sense. Still, it might be nice to be able to get the more detailed error message when available, and that means we should set the exception (OSError?) and return NULL.

And then there's the documentation and backward compatibility problem that Tim mentions. :( I'm not sure what the right answer is. Of the sigaction() errors, it seems that only EINTR should be possible to get from Python (since Python does its own sanity checking on the signal number, etc.).

I'm inclined to reject the patch unless it is modified to address the following issues:

• return SIG_ERR in all cases when an error occurs
• change the call locations to check for this error
  condition.
• preferrably include some test cases (either Python or using the CAPI test framework).
• document the new behavior in the C API manual

My vote: -1 for changing this in Python 2.2. Rejecting, and reassigning to Tim. Not closed. Changed the bug category.

[warsaw]

Logged In: YES user_id=12800

Alternatively, I've attached a quick fix that probably doesn't do enough, but might help things for Python 2.2

[tim-one]

Logged In: YES user_id=31435

Barry checked in an alternative that simply initializes context.sa_handler to SIG_ERR before calling sigaction.
This doesn't address the deeper API issues raised, but does address the original "uninitialized memory" bug, so closing this patch.