From 0e924c007dbb74f8f7dbdb75810c9b9a8ed6d3ec Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Mon, 8 Oct 2012 21:12:27 -0400 Subject: [PATCH] Fix lo_read, lo_write, lo_truncate to cope with "size_t" length parameters. libpq defines these functions as accepting "size_t" lengths ... but the underlying backend functions expect signed int32 length parameters, and so will misinterpret any value exceeding INT_MAX. Fix the libpq side to throw error rather than possibly doing something unexpected. This is a bug of long standing, but I doubt it's worth back-patching. The problem is really pretty academic anyway with lo_read/lo_write, since any caller expecting sane behavior would have to have provided a multi-gigabyte buffer. It's slightly more pressing with lo_truncate, but still we haven't supported large objects over 2GB until now. --- doc/src/sgml/lobj.sgml | 56 +++++++++++++++++++++++++++------- src/interfaces/libpq/fe-lobj.c | 50 +++++++++++++++++++++++++++--- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/lobj.sgml b/doc/src/sgml/lobj.sgml index ba5674cff37..db5bc100f8f 100644 --- a/doc/src/sgml/lobj.sgml +++ b/doc/src/sgml/lobj.sgml @@ -69,6 +69,17 @@ access reads and writes. </para> + <para> + The chunks stored for a large object do not have to be contiguous. + For example, if an application opens a new large object, seeks to offset + 1000000, and writes a few bytes there, this does not result in allocation + of 1000000 bytes worth of storage; only of chunks covering the range of + data bytes actually written. A read operation will, however, read out + zeroes for any unallocated locations preceding the last existing chunk. + This corresponds to the common behavior of <quote>sparsely allocated</> + files in <acronym>Unix</acronym> file systems. + </para> + <para> As of <productname>PostgreSQL</> 9.0, large objects have an owner and a set of access permissions, which can be managed using @@ -299,11 +310,19 @@ inv_fd = lo_open(conn, inv_oid, INV_READ|INV_WRITE); int lo_write(PGconn *conn, int fd, const char *buf, size_t len); </synopsis> writes <parameter>len</parameter> bytes from <parameter>buf</parameter> - to large object descriptor <parameter>fd</>. The <parameter>fd</parameter> - argument must have been returned by a previous - <function>lo_open</function>. The number of bytes actually - written is returned. In the event of an error, the return value - is -1. + (which must be of size <parameter>len</parameter>) to large object + descriptor <parameter>fd</>. The <parameter>fd</parameter> argument must + have been returned by a previous <function>lo_open</function>. The + number of bytes actually written is returned (in the current + implementation, this will always equal <parameter>len</parameter> unless + there is an error). In the event of an error, the return value is -1. +</para> + +<para> + Although the <parameter>len</parameter> parameter is declared as + <type>size_t</>, this function will reject length values larger than + <literal>INT_MAX</>. In practice, it's best to transfer data in chunks + of at most a few megabytes anyway. </para> </sect2> @@ -316,13 +335,22 @@ int lo_write(PGconn *conn, int fd, const char *buf, size_t len); <synopsis> int lo_read(PGconn *conn, int fd, char *buf, size_t len); </synopsis> - reads <parameter>len</parameter> bytes from large object descriptor - <parameter>fd</parameter> into <parameter>buf</parameter>. The - <parameter>fd</parameter> argument must have been returned by a - previous <function>lo_open</function>. The number of bytes - actually read is returned. In the event of an error, the return + reads up to <parameter>len</parameter> bytes from large object descriptor + <parameter>fd</parameter> into <parameter>buf</parameter> (which must be + of size <parameter>len</parameter>). The <parameter>fd</parameter> + argument must have been returned by a previous + <function>lo_open</function>. The number of bytes actually read is + returned; this will be less than <parameter>len</parameter> if the end of + the large object is reached first. In the event of an error, the return value is -1. </para> + +<para> + Although the <parameter>len</parameter> parameter is declared as + <type>size_t</>, this function will reject length values larger than + <literal>INT_MAX</>. In practice, it's best to transfer data in chunks + of at most a few megabytes anyway. +</para> </sect2> <sect2 id="lo-seek"> @@ -416,7 +444,7 @@ int lo_truncate(PGcon *conn, int fd, size_t len); <parameter>fd</parameter> argument must have been returned by a previous <function>lo_open</function>. If <parameter>len</> is greater than the large object's current length, the large object - is extended with null bytes ('\0'). + is extended to the specified length with null bytes ('\0'). On success, <function>lo_truncate</function> returns zero. On error, the return value is -1. </para> @@ -426,6 +454,12 @@ int lo_truncate(PGcon *conn, int fd, size_t len); <parameter>fd</parameter> is not changed. </para> +<para> + Although the <parameter>len</parameter> parameter is declared as + <type>size_t</>, <function>lo_truncate</function> will reject length + values larger than <literal>INT_MAX</>. +</para> + <para> <indexterm><primary>lo_truncate64</></> When dealing with large objects that might exceed 2GB in size, diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c index cf6fe3e128b..ffe333608c1 100644 --- a/src/interfaces/libpq/fe-lobj.c +++ b/src/interfaces/libpq/fe-lobj.c @@ -31,6 +31,7 @@ #endif #include <fcntl.h> +#include <limits.h> #include <sys/stat.h> #include <netinet/in.h> /* for ntohl/htonl */ #include <arpa/inet.h> @@ -155,13 +156,29 @@ lo_truncate(PGconn *conn, int fd, size_t len) return -1; } + /* + * Long ago, somebody thought it'd be a good idea to declare this function + * as taking size_t ... but the underlying backend function only accepts a + * signed int32 length. So throw error if the given value overflows + * int32. (A possible alternative is to automatically redirect the call + * to lo_truncate64; but if the caller wanted to rely on that backend + * function being available, he could have called lo_truncate64 for + * himself.) + */ + if (len > (size_t) INT_MAX) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("argument of lo_truncate exceeds integer range\n")); + return -1; + } + argv[0].isint = 1; argv[0].len = 4; argv[0].u.integer = fd; argv[1].isint = 1; argv[1].len = 4; - argv[1].u.integer = len; + argv[1].u.integer = (int) len; res = PQfn(conn, conn->lobjfuncs->fn_lo_truncate, &retval, &result_len, 1, argv, 2); @@ -251,13 +268,26 @@ lo_read(PGconn *conn, int fd, char *buf, size_t len) return -1; } + /* + * Long ago, somebody thought it'd be a good idea to declare this function + * as taking size_t ... but the underlying backend function only accepts a + * signed int32 length. So throw error if the given value overflows + * int32. + */ + if (len > (size_t) INT_MAX) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("argument of lo_read exceeds integer range\n")); + return -1; + } + argv[0].isint = 1; argv[0].len = 4; argv[0].u.integer = fd; argv[1].isint = 1; argv[1].len = 4; - argv[1].u.integer = len; + argv[1].u.integer = (int) len; res = PQfn(conn, conn->lobjfuncs->fn_lo_read, (int *) buf, &result_len, 0, argv, 2); @@ -293,15 +323,25 @@ lo_write(PGconn *conn, int fd, const char *buf, size_t len) return -1; } - if (len <= 0) - return 0; + /* + * Long ago, somebody thought it'd be a good idea to declare this function + * as taking size_t ... but the underlying backend function only accepts a + * signed int32 length. So throw error if the given value overflows + * int32. + */ + if (len > (size_t) INT_MAX) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("argument of lo_write exceeds integer range\n")); + return -1; + } argv[0].isint = 1; argv[0].len = 4; argv[0].u.integer = fd; argv[1].isint = 0; - argv[1].len = len; + argv[1].len = (int) len; argv[1].u.ptr = (int *) buf; res = PQfn(conn, conn->lobjfuncs->fn_lo_write, -- GitLab