Fix lo_read, lo_write, lo_truncate to cope with "size_t" length parameters.

tglsfdc · tglsfdc · commit 0e924c007dbb · 2012-10-08T21:19:53.000-04:00
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.
diff --git 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
@@ -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,
