Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

20201111 chaining db paged search #4434

Merged
merged 2 commits into from Nov 12, 2020

Conversation

Firstyear
Copy link
Contributor

This fixes #4428 by checking if the context we receive is NULL. This passes the reproduction case, and it also is shown under LSAN not to create any memory leaks or crashes with ASAN.

@Firstyear
Copy link
Contributor Author

note this has been split into two so that the fix can be applied for a customer hotfix.

@Firstyear Firstyear added this to the 1.4.3 milestone Nov 12, 2020
@Firstyear Firstyear self-assigned this Nov 12, 2020
Copy link
Contributor

@mreynolds389 mreynolds389 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides these minor nitpicks, you have my ack :)

pytestmark = pytest.mark.tier1

def test_chaining_paged_search(topology):
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case docstring parser may complain here because there is no text on the first line.

from lib389.idm.account import Accounts, Account
from lib389.topologies import topology_i2 as topology
from lib389.backend import Backends
from lib389._constants import *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, avoid mass imports if possible.

Bug Description: This test case shows how a paged search with criticality
set to false, causes chaining to sigsegv.

Fix Description: N/A - this is a reproducer, not the fix.

fixes: 389ds#4428

Author: William Brown <william@blackhats.net.au>

Review by: @droideck, @mreynolds389
…chaining

Bug Description: When a paged search through chaining backend is
received with a false criticality (such as SSSD), chaining backend
will sigsegv due to a null context.

Fix Description: When a NULL ctx is recieved to be freed, this is
as paged results have finished being sent, so we check the NULL
ctx and move on.

fixes: 389ds#4428

Author: William Brown <william@blackhats.net.au>

Review by: @droideck, @mreynolds389
@Firstyear Firstyear force-pushed the 20201111-chaining-db-paged-search branch from 0961b48 to db182f0 Compare November 12, 2020 22:55
@Firstyear Firstyear merged commit 7f241dc into 389ds:master Nov 12, 2020
@Firstyear Firstyear deleted the 20201111-chaining-db-paged-search branch November 12, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paged Results with noncritical request causes sigsegv in chaining backend
3 participants