bpo-41815: SQLite: segfault if backup called on closed database #22322

pdmccormick · 2020-09-19T16:11:39Z

bpo-41815: SQLite: fix segfault if backup called on closed database

Attempting to backup a closed database will trigger segfault:

import sqlite3
target = sqlite3.connect(':memory:')
source = sqlite3.connect(':memory:')
source.close()
source.backup(target)

https://bugs.python.org/issue41815

Automerge-Triggered-By: @aeros

the-knights-who-say-ni · 2020-09-19T16:11:42Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@pdmccormick

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

aeros

Thanks for the PR @pdmccormick, and welcome! The patch and test looks good to me, but it definitely could use a Misc/NEWS entry (see below for details).

I can confirm the segfault on master with the following (the original replication had a slight typo: "sqlite" -> "sqlite3" or it should be "import sqlite3 as sqlite" as is done in the source)

Python 3.10.0a0 (heads/master:c8c70e7876, Sep 19 2020, 21:28:47) 
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sqlite3
>>> target = sqlite3.connect(':memory:')
>>> source = sqlite3.connect(':memory:')
>>> source.close()
>>> source.backup(target)
[1]    17258 segmentation fault (core dumped)  ./python

The proposed changes in the PR addresses the segfault, and appropriately raises sqlite3.ProgrammingError

Python 3.10.0a0 (heads/bpo-41815-sqlite-closed-backup:9aee01c7be, Sep 19 2020, 22:06:20) 
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sqlite3
>>> target = sqlite3.connect(':memory:')
>>> source = sqlite3.connect(":memory:")
>>> source.close()
>>> source.backup(target)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
sqlite3.ProgrammingError: Cannot operate on a closed database.
>>>

I've also confirmed the segfault on the following versions: 3.9, 3.8. With 3.7 and below being security-only (or EoL far back enough), this fix should be backported only to 3.9 and 3.8.

However, I believe we do need approval from the release manager with both 3.9.0 and 3.8.6 currently being in the release candidate phase, and with this being a crash, this should be included IMO. I'll add Lukasz to the nosy list in the bpo issue, and elevate priority to release blocker.

Also, the placement of the additional check looks good to me, and the test sufficiently replicates the crash. The only thing that this is missing is a Misc/NEWS entry, which can be easily added with blurb it. Something succinct such as "Fix SQLite3 segfault when backing up closed database." should work (and optional attribution, such as "Contributed by <Name>."). A Misc/NEWS entry is generally necessary for any non-trivial changes, and I'd definitely consider this to be non-trivial (even if the surface area is small) since it fixes a segfault that's simple to replicate.

bedevere-bot · 2020-09-20T02:30:57Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

pdmccormick · 2020-09-20T03:19:15Z

Thank you for the review and guidance @aeros! I have made the requested changes; please review again.

bedevere-bot · 2020-09-20T03:19:18Z

Thanks for making the requested changes!

@aeros: please review the changes made to this pull request.

aeros

LGTM. I'll proceed with merging to master, but waiting on @ambv for the backports to 3.9 and 3.8. If some time passes and those branches are no longer in release candidate phase (see version PEPs for dates), feel free to ping me as a reminder to add the labels to trigger the automated backports.

miss-islington · 2020-09-21T21:41:49Z

Thanks @pdmccormick for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington · 2020-09-21T21:41:49Z

Thanks @pdmccormick for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

…onGH-22322) GH- [bpo-41815](): SQLite: fix segfault if backup called on closed database Attempting to backup a closed database will trigger segfault: ```python import sqlite3 target = sqlite3.connect(':memory:') source = sqlite3.connect(':memory:') source.close() source.backup(target) ``` (cherry picked from commit bfee9fa) Co-authored-by: Peter McCormick <[email protected]>

bedevere-bot · 2020-09-21T21:42:02Z

GH-22344 is a backport of this pull request to the 3.9 branch.

…onGH-22322) GH- [bpo-41815](): SQLite: fix segfault if backup called on closed database Attempting to backup a closed database will trigger segfault: ```python import sqlite3 target = sqlite3.connect(':memory:') source = sqlite3.connect(':memory:') source.close() source.backup(target) ``` (cherry picked from commit bfee9fa) Co-authored-by: Peter McCormick <[email protected]>

bedevere-bot · 2020-09-21T21:42:11Z

GH-22345 is a backport of this pull request to the 3.8 branch.

…2322) GH- [bpo-41815](): SQLite: fix segfault if backup called on closed database Attempting to backup a closed database will trigger segfault: ```python import sqlite3 target = sqlite3.connect(':memory:') source = sqlite3.connect(':memory:') source.close() source.backup(target) ``` (cherry picked from commit bfee9fa) Co-authored-by: Peter McCormick <[email protected]>

ambv · 2020-10-04T16:37:39Z

3.9 backport goes into 3.9.0.

…onGH-22322) GH- [bpo-41815](): SQLite: fix segfault if backup called on closed database Attempting to backup a closed database will trigger segfault: ```python import sqlite3 target = sqlite3.connect(':memory:') source = sqlite3.connect(':memory:') source.close() source.backup(target) ``` (cherry picked from commit bfee9fa) Co-authored-by: Peter McCormick <[email protected]>

…onGH-22322) # [bpo-41815](): SQLite: fix segfault if backup called on closed database Attempting to backup a closed database will trigger segfault: ```python import sqlite3 target = sqlite3.connect(':memory:') source = sqlite3.connect(':memory:') source.close() source.backup(target) ```

pdmccormick requested a review from berkerpeksag as a code owner September 19, 2020 16:11

the-knights-who-say-ni added the CLA not signed label Sep 19, 2020

bedevere-bot added the awaiting review label Sep 19, 2020

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Sep 19, 2020

aeros added the type-bug An unexpected behavior, bug, or error label Sep 20, 2020

aeros suggested changes Sep 20, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Sep 20, 2020

bpo-41815: SQLite: segfault if backup called on closed database

cba0abd

bedevere-bot added awaiting change review and removed awaiting changes labels Sep 20, 2020

bedevere-bot requested a review from aeros September 20, 2020 03:19

aeros approved these changes Sep 20, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Sep 20, 2020

aeros added the 🤖 automerge label Sep 20, 2020

miss-islington merged commit bfee9fa into python:master Sep 20, 2020

bedevere-bot removed the awaiting merge label Sep 20, 2020

aeros added needs backport to 3.8 needs backport to 3.9 only security fixes labels Sep 21, 2020

bedevere-bot removed the needs backport to 3.9 only security fixes label Sep 21, 2020

bedevere-bot removed the needs backport to 3.8 label Sep 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

bpo-41815: SQLite: segfault if backup called on closed database #22322

bpo-41815: SQLite: segfault if backup called on closed database #22322

Uh oh!

pdmccormick commented Sep 19, 2020 •

edited by miss-islington

Loading

Uh oh!

the-knights-who-say-ni commented Sep 19, 2020

Uh oh!

aeros left a comment

Uh oh!

bedevere-bot commented Sep 20, 2020

Uh oh!

pdmccormick commented Sep 20, 2020

Uh oh!

bedevere-bot commented Sep 20, 2020

Uh oh!

aeros left a comment

Uh oh!

miss-islington commented Sep 21, 2020

Uh oh!

miss-islington commented Sep 21, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

ambv commented Oct 4, 2020

Uh oh!

Uh oh!

Uh oh!

bpo-41815: SQLite: segfault if backup called on closed database #22322

bpo-41815: SQLite: segfault if backup called on closed database #22322

Uh oh!

Conversation

pdmccormick commented Sep 19, 2020 • edited by miss-islington Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

bpo-41815: SQLite: fix segfault if backup called on closed database

Uh oh!

the-knights-who-say-ni commented Sep 19, 2020

Recognized GitHub username

Uh oh!

aeros left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Sep 20, 2020

Uh oh!

pdmccormick commented Sep 20, 2020

Uh oh!

bedevere-bot commented Sep 20, 2020

Uh oh!

aeros left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Sep 21, 2020

Uh oh!

miss-islington commented Sep 21, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

ambv commented Oct 4, 2020

Uh oh!

Uh oh!

pdmccormick commented Sep 19, 2020 •

edited by miss-islington

Loading