Fix the converters between the old and new BIO_read functions to handle

igus68 · jogme · commit be42447469dc · 2026-02-12T09:34:25.000+01:00
end-of-file state properly. Related to openssl/project#1745 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> MergeDate: Thu Feb 12 08:34:31 2026 (Merged from #29290)
diff --git a/crypto/bio/bio_lib.c b/crypto/bio/bio_lib.c
@@ -254,10 +254,14 @@ int BIO_method_type(const BIO *b)
 }
 
 /*
- * This is essentially the same as BIO_read_ex() except that it allows
- * 0 or a negative value to indicate failure (retryable or not) in the return.
- * This is for compatibility with the old style BIO_read(), where existing code
- * may make assumptions about the return value that it might get.
+ * Internal BIO read function. Attempts to read dlen bytes from BIO b and
+ * places them in data. If any bytes were successfully read, then the number
+ * of bytes read is stored in readbytes.
+ * For compatibility with the old-style BIO_read() API, the function uses a
+ * return-value convention where a positive value indicates success,
+ * 0 indicates end-of-file, and a negative value indicates an error
+ * (including retryable errors).
+ * It also returns 0 if dlen==0.
  */
 static int bio_read_intern(BIO *b, void *data, size_t dlen, size_t *readbytes)
 {
@@ -285,6 +289,13 @@ static int bio_read_intern(BIO *b, void *data, size_t dlen, size_t *readbytes)
     if (ret > 0)
         b->num_read += (uint64_t)*readbytes;
 
+    /*
+     * If method->bread() returned 0 when dlen>0, it can be either EOF or
+     * an error, and we should distinguish them
+     */
+    if (ret == 0 && dlen > 0 && BIO_eof(b) != 1)
+        ret = -1;
+
     if (HAS_CALLBACK(b))
         ret = (int)bio_call_callback(b, BIO_CB_READ | BIO_CB_RETURN, data,
             dlen, 0, 0L, ret, readbytes);
@@ -303,8 +314,10 @@ int BIO_read(BIO *b, void *data, int dlen)
     size_t readbytes;
     int ret;
 
-    if (dlen < 0)
-        return 0;
+    if (dlen < 0) {
+        ERR_raise(ERR_LIB_BIO, ERR_R_PASSED_INVALID_ARGUMENT);
+        return -1;
+    }
 
     ret = bio_read_intern(b, data, (size_t)dlen, &readbytes);
 
@@ -679,6 +692,13 @@ long BIO_ctrl(BIO *b, int cmd, long larg, void *parg)
     return ret;
 }
 
+int BIO_eof(BIO *b)
+{
+    if ((b->flags & BIO_FLAGS_AUTO_EOF) != 0)
+        return 1;
+    return (int)BIO_ctrl(b, BIO_CTRL_EOF, 0, NULL);
+}
+
 long BIO_callback_ctrl(BIO *b, int cmd, BIO_info_cb *fp)
 {
     long ret;
diff --git a/crypto/bio/bio_meth.c b/crypto/bio/bio_meth.c
@@ -121,12 +121,25 @@ int bread_conv(BIO *bio, char *data, size_t datal, size_t *readbytes)
 {
     int ret;
 
+    if (datal == 0) {
+        *readbytes = 0;
+        return 1;
+    }
+
     if (datal > INT_MAX)
         datal = INT_MAX;
 
     ret = bio->method->bread_old(bio, data, (int)datal);
 
-    if (ret <= 0) {
+    bio->flags &= ~BIO_FLAGS_AUTO_EOF;
+    if (ret == 0) {
+        if (BIO_ctrl(bio, BIO_CTRL_EOF, 0, NULL) == 0)
+            bio->flags |= BIO_FLAGS_AUTO_EOF;
+        *readbytes = 0;
+        return 0;
+    }
+
+    if (ret < 0) {
         *readbytes = 0;
         return ret;
     }
diff --git a/doc/man3/BIO_meth_new.pod b/doc/man3/BIO_meth_new.pod
@@ -111,16 +111,35 @@ BIO_meth_get_write_ex() and BIO_meth_set_write_ex() get and set the function
 used for writing arbitrary length data to the BIO respectively. This function
 will be called in response to the application calling BIO_write_ex() or
 BIO_write(). The parameters for the function have the same meaning as for
-BIO_write_ex(). Older code may call BIO_meth_get_write() and
-BIO_meth_set_write() instead. Applications should not call both
-BIO_meth_set_write_ex() and BIO_meth_set_write() or call BIO_meth_get_write()
+BIO_write_ex() and it must return values as described for BIO_write_ex().
+
+Older code may call BIO_meth_get_write() and BIO_meth_set_write() instead
+to set an old-style write function. The parameters for the function have the
+same meaning as for BIO_write() and it must return values as described for
+BIO_write().
+
+Functions set by BIO_meth_set_write_ex() and BIO_meth_set_write() must call
+BIO_set_flags() to set the BIO_FLAGS_SHOULD_RETRY flag in relevant situations.
+
+Applications should not call both BIO_meth_set_write_ex() and
+BIO_meth_set_write() or call BIO_meth_get_write()
 when the function was set with BIO_meth_set_write_ex().
 
 BIO_meth_get_read_ex() and BIO_meth_set_read_ex() get and set the function used
 for reading arbitrary length data from the BIO respectively. This function will
 be called in response to the application calling BIO_read_ex() or BIO_read().
-The parameters for the function have the same meaning as for BIO_read_ex().
-Older code may call BIO_meth_get_read() and BIO_meth_set_read() instead.
+The parameters for the function have the same meaning as for BIO_read_ex()
+and it must return values as described for BIO_read_ex().
+The function must handle the end-of-file condition (if applicable) and return 0
+in this case.
+
+Older code may call BIO_meth_get_read() and BIO_meth_set_read() instead to
+set an old-style read function. The parameters for the function have the same
+meaning as for BIO_read() and it must return values as described for BIO_read().
+
+Functions set by BIO_meth_set_read_ex() and BIO_meth_set_read() must call
+BIO_set_flags() to set the BIO_FLAGS_SHOULD_RETRY flag in relevant situations.
+
 Applications should not call both BIO_meth_set_read_ex() and BIO_meth_set_read()
 or call BIO_meth_get_read() when the function was set with
 BIO_meth_set_read_ex().
@@ -141,6 +160,14 @@ processing ctrl messages in the BIO respectively. See the L<BIO_ctrl(3)> page fo
 more information. This function will be called in response to the application
 calling BIO_ctrl(). The parameters for the function have the same meaning as for
 BIO_ctrl().
+If the concept of end-of-file is meaningful for a BIO and the read method is
+set using BIO_meth_set_read_ex(), the ctrl function must handle the BIO_CTRL_EOF
+command and return an appropriate value (1 if EOF has been reached, 0 if not,
+or a negative value on failure), at least immediately after a read operation.
+If the read method is set using BIO_meth_set_read(), handling of the
+BIO_CTRL_EOF command is not mandatory; however, if such handling is implemented,
+it must return 1 if the read function returned 0 when attempting to read a
+nonzero number of bytes.
 
 BIO_meth_get_create() and BIO_meth_set_create() get and set the function used
 for creating a new instance of the BIO respectively. This function will be
diff --git a/doc/man3/BIO_read.pod b/doc/man3/BIO_read.pod
@@ -61,6 +61,11 @@ BIO_read_ex() returns 1 if data was successfully read, and 0 otherwise.
 BIO_write_ex() returns 1 if no error was encountered writing data, 0 otherwise.
 Requesting to write 0 bytes is not considered an error.
 
+BIO_read() returns the number of bytes read on success.
+A return value of 0 indicates that end-of-file was reached, or that a read
+of zero bytes was requested.
+A negative return value indicates an error condition.
+
 BIO_write() returns -2 if the "write" operation is not implemented by the BIO
 or -1 on other errors.
 Otherwise it returns the number of bytes written.
diff --git a/doc/man3/BIO_set_flags.pod b/doc/man3/BIO_set_flags.pod
@@ -112,13 +112,28 @@ Set if the last I/O operation on the B<BIO> should be retried at a later time.
 If this bit is not set then the condition is treated as an error.
 This flag is normally set by the B<BIO> implementation.
 
+=back
+
+The following flag can be used only with the B<base64 encoding BIO>.
+The result of using this flag with other BIOs is unpredictable.
+
+=over 4
+
 =item B<BIO_FLAGS_BASE64_NO_NL>
 
 When set on a base64 filter B<BIO> this flag disables the generation of
 newline characters in the encoded output and causes newlines to be ignored
 in the input.  See also L<BIO_f_base64(3)>.
 The flag has no effect on any other built-in B<BIO> types.
 
+=back
+
+The following flags can be used only with the B<memory buffer BIO> and
+B<secure memory buffer BIO>. The result of using these flags with other BIOs
+is unpredictable.
+
+=over 4
+
 =item B<BIO_FLAGS_MEM_RDONLY>
 
 When set on a memory B<BIO> this flag indicates that the underlying buffer is
@@ -134,12 +149,8 @@ The flag has no effect on any other built-in B<BIO> types.
 
 =item B<BIO_FLAGS_IN_EOF>
 
-This flag may be used by a B<BIO> implementation to indicate that the end
-of the input stream has been reached.  However, B<BIO> types are not
-required to use this flag to signal end-of-file conditions; they may rely
-on other mechanisms such as system calls or by querying the next B<BIO> in a
-chain.  Applications must therefore not test this flag directly to
-determine whether EOF has been reached, and must use BIO_eof() instead.
+This flag is for internal use only. It should not be used outside BIO
+implementations.
 
 =back
 
diff --git a/include/internal/bio.h b/include/internal/bio.h
@@ -44,6 +44,10 @@ int bread_conv(BIO *bio, char *data, size_t datal, size_t *read);
 #define BIO_CTRL_CLEAR_KTLS_TX_CTRL_MSG 75
 #define BIO_CTRL_SET_KTLS_TX_ZEROCOPY_SENDFILE 90
 
+/* Internal BIO flags */
+
+#define BIO_FLAGS_AUTO_EOF 0x80
+
 /*
  * This is used with memory BIOs:
  * BIO_FLAGS_MEM_LEGACY_EOF means legacy behaviour of BIO_eof()
diff --git a/include/openssl/bio.h.in b/include/openssl/bio.h.in
@@ -230,6 +230,17 @@ extern "C" {
 #define BIO_FLAGS_UPLINK 0
 #endif
 
+/* the BIO FLAGS values 0x10 to 0x80 are reserved for internal use */
+
+/*
+ * BIO FLAGS in the range 0x0100..0x8000 are BIO-type specific.
+ * Their meaning is defined by the particular BIO implementation and
+ * is not shared across different BIO types. The same bit value may
+ * have a different meaning or no meaning at all in other BIOs.
+ * Such flags may be part of the public API or internal to the BIO.
+ */
+
+/* This is used with base64 BIO */
 #define BIO_FLAGS_BASE64_NO_NL 0x100
 
 /*
@@ -241,8 +252,6 @@ extern "C" {
 #define BIO_FLAGS_NONCLEAR_RST 0x400
 #define BIO_FLAGS_IN_EOF 0x800
 
-/* the BIO FLAGS values 0x1000 to 0x8000 are reserved for internal KTLS flags */
-
 typedef union bio_addr_st BIO_ADDR;
 typedef struct bio_addrinfo_st BIO_ADDRINFO;
 
@@ -607,7 +616,6 @@ int BIO_read_filename(BIO *b, const char *name);
 #define BIO_dup_state(b, ret) BIO_ctrl(b, BIO_CTRL_DUP, 0, (char *)(ret))
 
 #define BIO_reset(b) (int)BIO_ctrl(b, BIO_CTRL_RESET, 0, NULL)
-#define BIO_eof(b) (int)BIO_ctrl(b, BIO_CTRL_EOF, 0, NULL)
 #define BIO_set_close(b, c) (int)BIO_ctrl(b, BIO_CTRL_SET_CLOSE, (c), NULL)
 #define BIO_get_close(b) (int)BIO_ctrl(b, BIO_CTRL_GET_CLOSE, 0, NULL)
 #define BIO_pending(b) (int)BIO_ctrl(b, BIO_CTRL_PENDING, 0, NULL)
@@ -733,6 +741,7 @@ __owur int BIO_get_wpoll_descriptor(BIO *b, BIO_POLL_DESCRIPTOR *desc);
 int BIO_puts(BIO *bp, const char *buf);
 int BIO_indent(BIO *b, int indent, int max);
 long BIO_ctrl(BIO *bp, int cmd, long larg, void *parg);
+int BIO_eof(BIO *b);
 long BIO_callback_ctrl(BIO *b, int cmd, BIO_info_cb *fp);
 void *BIO_ptr_ctrl(BIO *bp, int cmd, long larg);
 long BIO_int_ctrl(BIO *bp, int cmd, long larg, int iarg);
diff --git a/test/bio_callback_test.c b/test/bio_callback_test.c
@@ -13,7 +13,7 @@
 
 #include "testutil.h"
 
-#define MAXCOUNT 5
+#define MAXCOUNT 7
 static int my_param_count;
 static BIO *my_param_b[MAXCOUNT];
 static int my_param_oper[MAXCOUNT];
@@ -138,20 +138,20 @@ static int test_bio_callback_ex(void)
     my_param_count = 0;
     i = BIO_read(bio, buf, sizeof(buf));
     if (!TEST_int_eq(i, 0)
-        || !TEST_int_eq(my_param_count, 2)
+        || !TEST_int_eq(my_param_count, 6)
         || !TEST_ptr_eq(my_param_b[0], bio)
         || !TEST_int_eq(my_param_oper[0], BIO_CB_READ)
         || !TEST_ptr_eq(my_param_argp[0], buf)
         || !TEST_size_t_eq(my_param_len[0], sizeof(buf))
         || !TEST_long_eq(my_param_argl[0], 0L)
         || !TEST_int_eq((int)my_param_ret[0], 1)
-        || !TEST_ptr_eq(my_param_b[1], bio)
-        || !TEST_int_eq(my_param_oper[1], BIO_CB_READ | BIO_CB_RETURN)
-        || !TEST_ptr_eq(my_param_argp[1], buf)
-        || !TEST_size_t_eq(my_param_len[1], sizeof(buf))
-        || !TEST_long_eq(my_param_argl[1], 0L)
-        || !TEST_size_t_eq(my_param_processed[1], 0)
-        || !TEST_int_eq((int)my_param_ret[1], 0))
+        || !TEST_ptr_eq(my_param_b[5], bio)
+        || !TEST_int_eq(my_param_oper[5], BIO_CB_READ | BIO_CB_RETURN)
+        || !TEST_ptr_eq(my_param_argp[5], buf)
+        || !TEST_size_t_eq(my_param_len[5], sizeof(buf))
+        || !TEST_long_eq(my_param_argl[5], 0L)
+        || !TEST_size_t_eq(my_param_processed[5], 0)
+        || !TEST_int_eq((int)my_param_ret[5], 0))
         goto err;
 
     my_param_count = 0;
@@ -291,19 +291,19 @@ static int test_bio_callback(void)
     my_param_count = 0;
     i = BIO_read(bio, buf, sizeof(buf));
     if (!TEST_int_eq(i, 0)
-        || !TEST_int_eq(my_param_count, 2)
+        || !TEST_int_eq(my_param_count, 6)
         || !TEST_ptr_eq(my_param_b[0], bio)
         || !TEST_int_eq(my_param_oper[0], BIO_CB_READ)
         || !TEST_ptr_eq(my_param_argp[0], buf)
         || !TEST_int_eq(my_param_argi[0], sizeof(buf))
         || !TEST_long_eq(my_param_argl[0], 0L)
         || !TEST_long_eq(my_param_ret[0], 1L)
-        || !TEST_ptr_eq(my_param_b[1], bio)
-        || !TEST_int_eq(my_param_oper[1], BIO_CB_READ | BIO_CB_RETURN)
-        || !TEST_ptr_eq(my_param_argp[1], buf)
-        || !TEST_int_eq(my_param_argi[1], sizeof(buf))
-        || !TEST_long_eq(my_param_argl[1], 0L)
-        || !TEST_long_eq(my_param_ret[1], 0L))
+        || !TEST_ptr_eq(my_param_b[5], bio)
+        || !TEST_int_eq(my_param_oper[5], BIO_CB_READ | BIO_CB_RETURN)
+        || !TEST_ptr_eq(my_param_argp[5], buf)
+        || !TEST_int_eq(my_param_argi[5], sizeof(buf))
+        || !TEST_long_eq(my_param_argl[5], 0L)
+        || !TEST_long_eq(my_param_ret[5], 0L))
         goto err;
 
     my_param_count = 0;
diff --git a/test/bio_eof_test.c b/test/bio_eof_test.c
diff --git a/test/build.info b/test/build.info
diff --git a/test/recipes/61-test_bio_eof.t b/test/recipes/61-test_bio_eof.t
diff --git a/util/libcrypto.num b/util/libcrypto.num
