Bug 14065 - PIDL generated Python bindings get talloc references wrong
Summary: PIDL generated Python bindings get talloc references wrong
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Python (show other bugs)
Version: 4.10.6
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Jo Sutton
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-02 05:00 UTC by Douglas Bagnall
Modified: 2022-08-12 00:20 UTC (History)
1 user (show)

See Also:


Attachments
eight crashes from referencing but not calling python object attributes (247.05 KB, application/gzip)
2019-08-10 02:02 UTC, Douglas Bagnall
no flags Details
patch for 4.13 (4.13 KB, patch)
2021-06-01 02:37 UTC, Jo Sutton
no flags Details
patch for 4.14 (4.13 KB, patch)
2021-06-01 02:37 UTC, Jo Sutton
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Douglas Bagnall 2019-08-02 05:00:08 UTC
The generated bindings assume that inline arrays are talloc contexts, which is never true.

For example, in the IDL we have

	typedef struct {
		DWORD           dwRpcStructureVersion;
		DWORD           dwReserved0;

		[string, charset(UTF8)] char *          pszDpFqdn;
		[string, charset(UTF16)] wchar_t *       pszDpDn;
		[string, charset(UTF16)] wchar_t *       pszCrDn;
		DWORD           dwFlags;
		DWORD           dwZoneCount;
		DNS_DP_STATE    dwState;

		DWORD           dwReserved[    3 ];
		DNS_EXTENSION   pwszReserved[ 3 ];
		[range(0,10000)]               DWORD                    dwReplicaCount;
		[size_is(dwReplicaCount)]      PDNS_RPC_DP_REPLICA      ReplicaArray[];
	}
	DNS_RPC_DP_INFO;


where `DNS_EXTENSION   pwszReserved[ 3 ];` becomes a struct field, not a pointer to an array, but in  
bin/default/librpc/gen_ndr/py_dnsserver.c we try using 'object->pwszReserved' as a talloc context:

static PyObject *py_DNS_RPC_DP_INFO_get_pwszReserved(PyObject *obj, void *closure)
{
	struct DNS_RPC_DP_INFO *object = (struct DNS_RPC_DP_INFO *)pytalloc_get_ptr(obj);
	PyObject *py_pwszReserved;
	py_pwszReserved = PyList_New(3);
	if (py_pwszReserved == NULL) {
		return NULL;
	}
	{
		int pwszReserved_cntr_0;
		for (pwszReserved_cntr_0 = 0; pwszReserved_cntr_0 < (3); pwszReserved_cntr_0++) {
			PyObject *py_pwszReserved_0;
			py_pwszReserved_0 = pytalloc_reference_ex(&DNS_EXTENSION_Type, object->pwszReserved, &object->pwszReserved[pwszReserved_cntr_0]);
			PyList_SetItem(py_pwszReserved, pwszReserved_cntr_0, py_pwszReserved_0);
		}
	}
	return py_pwszReserved;
}


With this result (showing get and set):


$ python3 -c'from samba.dcerpc import dnsserver;dnsserver.DNS_RPC_DP_INFO().pwszReserved'
Bad talloc magic value - unknown value
Aborted (core dumped)

$ python3 -c'from samba.dcerpc import dnsserver;x=dnsserver.DNS_RPC_DP_INFO();a=dnsserver.DNS_EXTENSION();x.pwszReserved = [a, None, None]'
Bad talloc magic value - unknown value
Aborted (core dumped)


It seems like Python.pm needs to be extended to carry around the last know good talloc pointer as distinct from  $var_name which it currently uses.
Comment 1 Douglas Bagnall 2019-08-10 02:02:02 UTC
Created attachment 15389 [details]
eight crashes from referencing but not calling python object attributes
Comment 2 Samba QA Contact 2021-05-28 09:51:03 UTC
This bug was referenced in samba master:

9019e08c61a9b0bfce9ef295089ad42e46e784e2
537f2d19b5513fa9c29034bc53958a2c05768e81
Comment 3 Andrew Bartlett 2021-05-31 03:07:32 UTC
Joseph,

Can you see if it would make sense to backport this, say to support Dogulas' work?
Comment 4 Jo Sutton 2021-06-01 02:37:01 UTC
Created attachment 16636 [details]
patch for 4.13
Comment 5 Jo Sutton 2021-06-01 02:37:40 UTC
Created attachment 16637 [details]
patch for 4.14
Comment 6 Douglas Bagnall 2022-08-12 00:20:56 UTC
Fixed in master. It looks like backports missed, but that's ok now.