Bug 680269 - RETURN_STRING in python-ethtool-0.6 doesn't correctly handle the reference count of the Py_None singleton
Summary: RETURN_STRING in python-ethtool-0.6 doesn't correctly handle the reference co...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: python-ethtool
Version: 6.1
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Dave Malcolm
QA Contact: Petr Šplíchal
URL:
Whiteboard:
Depends On:
Blocks: 605533
TreeView+ depends on / blocked
 
Reported: 2011-02-24 20:53 UTC by Dave Malcolm
Modified: 2016-06-01 01:41 UTC (History)
3 users (show)

Fixed In Version: python-ethtool-0.6-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-05-19 14:25:16 UTC
Target Upstream Version:


Attachments (Terms of Use)
RETURN_STRING() did not use Py_INCREF() when returning Py_None (1.31 KB, text/plain)
2011-02-25 12:04 UTC, David Sommerseth
no flags Details
Improved error situations in case of NULL returns (2.08 KB, text/plain)
2011-02-25 12:05 UTC, David Sommerseth
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:0770 0 normal SHIPPED_LIVE python-ethtool bug fix and enhancement update 2011-05-18 18:08:35 UTC

Description Dave Malcolm 2011-02-24 20:53:49 UTC
Description of problem:
Looking at e.g.:
http://fedorapeople.org/gitweb?p=dsommers/public_git/python-ethtool.git;a=blob;f=python- ethtool/etherinfo_obj.c;h=04eb634a4e0fababf3369be649fcc039d2f791bb;hb=HEAD

I see e.g. _ethtool_etherinfo_getter using the RETURN_STRING macro.

This is declared in:
http://fedorapeople.org/gitweb?p=dsommers/public_git/python-ethtool.git;a=blob;f=python-ethtool/etherinfo_struct.h;h=b1d453ffae8b282386e60cb60ed9d31360b1701c;hb=HEAD

  #define RETURN_STRING(str) (str ? PyString_FromString(str) : Py_None)

This isn't incrementing the reference count on the Py_None singleton when it should be (the caller assumes that it "owns" a ref on the result of _getter, and will decref it), it could cause the python process to bail out:
  "Fatal Python error: deallocating None"
if run repeatedly.

However, this doesn't seem to present in the 0.3 currently in RHEL6.

(There also seem to be a few places in python-ethtool/etherinfo_obj.c:_ethtool_etherinfo_get_ipv6_addresses where the code assumes that certain Py_ API calls don't return NULL)

Filing since bug 605533 proposes a rebase to 0.6

Comment 2 Dave Malcolm 2011-02-24 21:55:36 UTC
>>> for i in range(10000): ethtool.get_interfaces_info('lo')[0].ipv4_broadcast
... 
Fatal Python error: deallocating None
Aborted (core dumped)

This was with a local rebuild of python-ethtool-0.6-0.el6.x86_64

Comment 3 Dave Malcolm 2011-02-25 00:38:36 UTC
Note that Python/object.h has:
   /* Macro for returning Py_None from a function */
   #define Py_RETURN_NONE return Py_INCREF(Py_None), Py_None

so either that could be used, or adapted to something like:
  #define RETURN_STRING(str) \
      (str ? \
       PyString_FromString(str) : \
       (Py_INCREF(Py_None), Py_None) \
      )

though I haven't tested that code.

Comment 4 David Sommerseth 2011-02-25 12:04:39 UTC
Created attachment 480977 [details]
RETURN_STRING() did not use Py_INCREF() when returning Py_None

From aa2c20e697af1907b92129410aa10952a3ffdd68 Mon Sep 17 00:00:00 2001
From: David Sommerseth <davids>
Date: Fri, 25 Feb 2011 09:57:04 +0100
Subject: [PATCH 1/2] RETURN_STRING() did not use Py_INCREF() when returning Py_None

From RH BZ#680269:

     #define RETURN_STRING(str) (str ? PyString_FromString(str) : Py_None)

  This isn't incrementing the reference count on the Py_None singleton when it
  should be (the caller assumes that it "owns" a ref on the result of _getter,
  and will decref it), it could cause the python process to bail out:
    "Fatal Python error: deallocating None"
  if run repeatedly.

Reported-by: Dave Malcolm <dmalcolm>
Signed-off-by: David Sommerseth <davids>
---
 python-ethtool/etherinfo_struct.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comment 5 David Sommerseth 2011-02-25 12:05:24 UTC
Created attachment 480978 [details]
Improved error situations in case of NULL returns

From 4e928d62a8e3c1dfefe6a55b81c0f0b4510b14eb Mon Sep 17 00:00:00 2001
From: David Sommerseth <davids>
Subject: [PATCH 2/2] Improved error situations in case of NULL returns

_ethtool_etherinfo_get_ipv6_addresses() didn't check too well several
Python calls if it would return NULL.

Reported-by: Dave Malcolm <dmalcolm>
Signed-off-by: David Sommerseth <davids>
---
 python-ethtool/etherinfo_obj.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

Comment 9 errata-xmlrpc 2011-05-19 14:25:16 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0770.html


Note You need to log in before you can comment on or make changes to this bug.