Skip to content

Commit faa54a3

Browse files
nnorwitznnorwitz
committed
Code review of the new buffer protocol. Mostly add questions that should
be answered with the comments removed. There are many places that require checks when doing arithmetic for memory sizes when allocating memory. Otherwise, overflow is possible with a subsequent crash. Fix SF #1777057 which was a result of not initializing the new BufferError properly. Had to update the test for exceptions for BufferError too.
1 parent 1836358 commit faa54a3

9 files changed

Lines changed: 29 additions & 20 deletions

File tree

‎Include/bytesobject.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ extern "C" {
2121
/* Object layout */
2222
typedef struct {
2323
PyObject_VAR_HEAD
24+
/* XXX(nnorwitz): should ob_exports be Py_ssize_t? */
2425
int ob_exports; /* how many buffer exports */
2526
Py_ssize_t ob_alloc; /* How many bytes allocated */
2627
char *ob_bytes;

‎Include/object.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ typedef struct bufferinfo {
147147
Py_ssize_t len;
148148
Py_ssize_t itemsize;
149149
int readonly;
150-
int ndim;
150+
int ndim; /* XXX(nnorwitz): should be Py_ssize_t??? */
151151
char *format;
152152
Py_ssize_t *shape;
153153
Py_ssize_t *strides;

‎Lib/test/exception_hierarchy.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ BaseException
1010
| +-- ZeroDivisionError
1111
+-- AssertionError
1212
+-- AttributeError
13+
+-- BufferError
1314
+-- EnvironmentError
1415
| +-- IOError
1516
| +-- OSError

‎Modules/arraymodule.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,8 @@ static PyTypeObject Arraytype;
4242

4343
#ifdef Py_UNICODE_WIDE
4444
#define PyArr_UNI 'w'
45-
/*static const char *PyArr_UNISTR = "w"; */
4645
#else
4746
#define PyArr_UNI 'u'
48-
/*static const char *PyArr_UNISTR = "u"; */
4947
#endif
5048

5149
#define array_Check(op) PyObject_TypeCheck(op, &Arraytype)
@@ -1773,6 +1771,8 @@ array_buffer_getbuf(arrayobject *self, PyBuffer *view, int flags)
17731771
view->internal = NULL;
17741772
if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) {
17751773
view->internal = malloc(3);
1774+
/* XXX(nnorwitz): need to check for malloc failure.
1775+
Should probably use PyObject_Malloc. */
17761776
view->format = view->internal;
17771777
view->format[0] = (char)(self->ob_descr->typecode);
17781778
view->format[1] = '\0';

‎Objects/abstract.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ PyBuffer_ToContiguous(void *buf, PyBuffer *view, Py_ssize_t len, char fort)
471471

472472
/* Otherwise a more elaborate scheme is needed */
473473

474+
/* XXX(nnorwitz): need to check for overflow! */
474475
indices = (Py_ssize_t *)PyMem_Malloc(sizeof(Py_ssize_t)*(view->ndim));
475476
if (indices == NULL) {
476477
PyErr_NoMemory();
@@ -521,6 +522,7 @@ PyBuffer_FromContiguous(PyBuffer *view, void *buf, Py_ssize_t len, char fort)
521522

522523
/* Otherwise a more elaborate scheme is needed */
523524

525+
/* XXX(nnorwitz): need to check for overflow! */
524526
indices = (Py_ssize_t *)PyMem_Malloc(sizeof(Py_ssize_t)*(view->ndim));
525527
if (indices == NULL) {
526528
PyErr_NoMemory();
@@ -594,6 +596,7 @@ int PyObject_CopyData(PyObject *dest, PyObject *src)
594596

595597
/* Otherwise a more elaborate copy scheme is needed */
596598

599+
/* XXX(nnorwitz): need to check for overflow! */
597600
indices = (Py_ssize_t *)PyMem_Malloc(sizeof(Py_ssize_t)*view_src.ndim);
598601
if (indices == NULL) {
599602
PyErr_NoMemory();
@@ -606,6 +609,7 @@ int PyObject_CopyData(PyObject *dest, PyObject *src)
606609
}
607610
elements = 1;
608611
for (k=0; k<view_src.ndim; k++) {
612+
/* XXX(nnorwitz): can this overflow? */
609613
elements *= view_src.shape[k];
610614
}
611615
while (elements--) {

‎Objects/bufferobject.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ buffer_releasebuf(PyBufferObject *self, PyBuffer *view)
6464
(*bp->bf_releasebuffer)(self->b_base, view);
6565
}
6666
}
67-
return;
67+
/* XXX(nnorwitz): do we need to release view here? it leaks. */
6868
}
6969

7070
static PyObject *
@@ -401,6 +401,7 @@ buffer_concat(PyBufferObject *self, PyObject *other)
401401
return NULL;
402402
}
403403

404+
/* XXX(nnorwitz): need to check for overflow! */
404405
ob = PyBytes_FromStringAndSize(NULL, view.len+view2.len);
405406
if ( ob == NULL ) {
406407
PyObject_ReleaseBuffer((PyObject *)self, &view);
@@ -427,6 +428,7 @@ buffer_repeat(PyBufferObject *self, Py_ssize_t count)
427428
count = 0;
428429
if (!get_buf(self, &view, PyBUF_SIMPLE))
429430
return NULL;
431+
/* XXX(nnorwitz): need to check for overflow! */
430432
ob = PyBytes_FromStringAndSize(NULL, view.len * count);
431433
if ( ob == NULL )
432434
return NULL;
@@ -474,6 +476,7 @@ buffer_slice(PyBufferObject *self, Py_ssize_t left, Py_ssize_t right)
474476
right = view.len;
475477
if ( right < left )
476478
right = left;
479+
/* XXX(nnorwitz): is it possible to access unitialized memory? */
477480
ob = PyBytes_FromStringAndSize((char *)view.buf + left,
478481
right - left);
479482
PyObject_ReleaseBuffer((PyObject *)self, &view);

‎Objects/bytesobject.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ bytes_setslice(PyBytesObject *self, Py_ssize_t lo, Py_ssize_t hi,
507507
memmove(self->ob_bytes + lo + needed, self->ob_bytes + hi,
508508
Py_Size(self) - hi);
509509
}
510+
/* XXX(nnorwitz): need to verify this can't overflow! */
510511
if (PyBytes_Resize((PyObject *)self,
511512
Py_Size(self) + needed - avail) < 0) {
512513
res = -1;

‎Objects/exceptions.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,6 +1694,7 @@ _PyExc_Init(void)
16941694
PRE_INIT(ZeroDivisionError)
16951695
PRE_INIT(SystemError)
16961696
PRE_INIT(ReferenceError)
1697+
PRE_INIT(BufferError)
16971698
PRE_INIT(MemoryError)
16981699
PRE_INIT(Warning)
16991700
PRE_INIT(UserWarning)
@@ -1753,6 +1754,7 @@ _PyExc_Init(void)
17531754
POST_INIT(ZeroDivisionError)
17541755
POST_INIT(SystemError)
17551756
POST_INIT(ReferenceError)
1757+
POST_INIT(BufferError)
17561758
POST_INIT(MemoryError)
17571759
POST_INIT(Warning)
17581760
POST_INIT(UserWarning)

‎Objects/memoryobject.c‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ Create a new memoryview object which references the given object.");
2626
PyObject *
2727
PyMemoryView_FromMemory(PyBuffer *info)
2828
{
29+
/* XXX(nnorwitz): need to implement something here? */
2930
return NULL;
3031
}
3132

@@ -59,7 +60,7 @@ static PyObject *
5960
memory_new(PyTypeObject *subtype, PyObject *args, PyObject *kwds)
6061
{
6162
PyObject *obj;
62-
if (!PyArg_ParseTuple(args, "O", &obj)) return NULL;
63+
if (!PyArg_UnpackTuple(args, "memoryview", 1, 1, &obj)) return NULL;
6364

6465
return PyMemoryView_FromObject(obj);
6566
}
@@ -136,6 +137,7 @@ _indirect_copy_nd(char *dest, PyBuffer *view, char fort)
136137
void (*func)(int, Py_ssize_t *, Py_ssize_t *);
137138

138139

140+
/* XXX(nnorwitz): need to check for overflow! */
139141
indices = (Py_ssize_t *)PyMem_Malloc(sizeof(Py_ssize_t)*view->ndim);
140142
if (indices == NULL) {
141143
PyErr_NoMemory();
@@ -260,6 +262,7 @@ PyMemoryView_GetContiguous(PyObject *obj, int buffertype, char fort)
260262
/* return a shadowed memory-view object */
261263
view->buf = dest;
262264
mem->base = PyTuple_Pack(2, obj, bytes);
265+
/* XXX(nnorwitz): need to verify alloc was successful. */
263266
Py_DECREF(bytes);
264267
}
265268
else {
@@ -373,34 +376,31 @@ static PyGetSetDef memory_getsetlist[] ={
373376

374377

375378
static PyObject *
376-
memory_tobytes(PyMemoryViewObject *mem, PyObject *args)
379+
memory_tobytes(PyMemoryViewObject *mem, PyObject *noargs)
377380
{
378-
if (!PyArg_ParseTuple(args, "")) return NULL;
379381
/* Create new Bytes object for data */
380382
return PyBytes_FromObject((PyObject *)mem);
381383
}
382384

383385
static PyObject *
384-
memory_tolist(PyMemoryViewObject *mem, PyObject *args)
386+
memory_tolist(PyMemoryViewObject *mem, PyObject *noargs)
385387
{
386-
if (!PyArg_ParseTuple(args, "")) return NULL;
387388
Py_INCREF(Py_NotImplemented);
388389
return Py_NotImplemented;
389390
}
390391

391392

392393

393394
static PyMethodDef memory_methods[] = {
394-
{"tobytes", (PyCFunction)memory_tobytes, 1, NULL},
395-
{"tolist", (PyCFunction)memory_tolist, 1, NULL},
395+
{"tobytes", (PyCFunction)memory_tobytes, METH_NOARGS, NULL},
396+
{"tolist", (PyCFunction)memory_tolist, METH_NOARGS, NULL},
396397
{NULL, NULL} /* sentinel */
397398
};
398399

399400

400401
static void
401402
memory_dealloc(PyMemoryViewObject *self)
402403
{
403-
404404
if (PyTuple_Check(self->base)) {
405405
/* Special case when first element is generic object
406406
with buffer interface and the second element is a
@@ -423,21 +423,18 @@ memory_dealloc(PyMemoryViewObject *self)
423423
else {
424424
PyObject_ReleaseBuffer(self->base, &(self->view));
425425
}
426-
Py_DECREF(self->base);
426+
Py_CLEAR(self->base);
427427
PyObject_DEL(self);
428428
}
429429

430430
static PyObject *
431431
memory_repr(PyMemoryViewObject *self)
432432
{
433-
434-
if ( self->base == NULL )
435-
return PyUnicode_FromFormat("<memory at %p>",
436-
self);
433+
/* XXX(nnorwitz): the code should be different or remove condition. */
434+
if (self->base == NULL)
435+
return PyUnicode_FromFormat("<memory at %p>", self);
437436
else
438-
return PyUnicode_FromFormat(
439-
"<memory at %p>",
440-
self);
437+
return PyUnicode_FromFormat("<memory at %p>", self);
441438
}
442439

443440

0 commit comments

Comments
 (0)