Skip to content

Python bindings: fix use-after-free on field definitions - #15288

Open
alonfaraj wants to merge 2 commits into
OSGeo:masterfrom
alonfaraj:fix/python-bindings-featuredefn-refs
Open

alonfaraj wants to merge 2 commits into
OSGeo:masterfrom
alonfaraj:fix/python-bindings-featuredefn-refs

Conversation

@alonfaraj

Copy link
Copy Markdown
Contributor

What does this PR do?

GetLayerDefn() and GetDefnRef() already hold a referene to the FeatureDefn, but the field defns you get out of it don't, so once the dataset or feature goes away they point at freed memory:

from osgeo import ogr
ogr.UseExceptions()

def first_field(path):
    ds = ogr.Open(path)
    return ds.GetLayer(0).GetLayerDefn().GetFieldDefn(0)

print(first_field("poly.shp").GetName())   # garbage instead of 'AREA'

Same thing with GetGeomFieldDefn(0) - segfaults when calling GetSpatialRef().

Reproduced 3.13.3 and master.

Fix

Make returned objects keep a Python reference to their parent:

  • FeatureDefn.GetFieldDefn / GetGeomFieldDefn keep FeatureDefn
  • Feature.GetFieldDefnRef / GetGeomFieldDefnRef keep Feature
  • GeomFieldDefn.GetCoordinatePrecision keep GeomFieldDefn

Tests

Added test_field_defn_use_after_layer_del and test_field_defn_use_after_feature_del

What are related issues/pull requests?

AI tool usage

  • AI (Y-a-t-il-un-Copilot-dans-l'avion, Chat-j'ai-pété, Jean-Claude Dusse or something similar) supported my development of this PR. See our policy about AI tool use. Use of AI tools must be indicated.

Tasklist

  • Make sure code is correctly formatted (cf pre-commit configuration)
  • Add test case(s)
  • Add documentation
  • Updated Python API documentation (swig/include/python/docs/)
  • Review
  • Adjust for comments
  • All CI builds and checks have passed
  • ADD YOUR TASKS HERE

Environment

Provide environment details, if relevant:

  • OS: Ubuntu 26.04
  • Compiler: gcc (Ubuntu 15.2.0-16ubuntu1) 15.2.0

This branch has not been deployed

No deployments
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.

1 participant