diff --git a/library/mps_reader.c b/library/mps_reader.c index ffe19dd27..9f08c5267 100644 --- a/library/mps_reader.c +++ b/library/mps_reader.c @@ -350,53 +350,14 @@ int mbedtls_mps_reader_get( mbedtls_mps_reader *rd, int mbedtls_mps_reader_commit( mbedtls_mps_reader *rd ) { - unsigned char *acc; - mbedtls_mps_size_t aa, end, fo, shift; + mbedtls_mps_size_t end; MBEDTLS_MPS_TRACE_INIT( "reader_commit" ); - MBEDTLS_MPS_STATE_VALIDATE_RAW( rd->frag != NULL, "mbedtls_mps_reader_commit() requires reader to be in consuming mode" ); - acc = rd->acc; end = rd->end; - - if( acc == NULL ) - { - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "No accumulator, just shift end" ); - rd->commit = end; - MBEDTLS_MPS_TRACE_RETURN( 0 ); - } - - fo = rd->acc_share.frag_offset; - if( end >= fo ) - { - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "Started to serve fragment, get rid of accumulator" ); - shift = fo; - aa = 0; - } - else - { - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "Still serving from accumulator" ); - aa = rd->acc_avail; - shift = end; - memmove( acc, acc + shift, aa - shift ); - aa -= shift; - } - - end -= shift; - fo -= shift; - - rd->acc_share.frag_offset = fo; - rd->acc_avail = aa; rd->commit = end; - rd->end = end; - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "Final state: (end=commit,fo,avail) = (%u,%u,%u)", - (unsigned) end, (unsigned) fo, (unsigned) aa ); MBEDTLS_MPS_TRACE_RETURN( 0 ); } @@ -406,7 +367,7 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *rd, unsigned char *frag, *acc; mbedtls_mps_size_t pending, commit; mbedtls_mps_size_t al, fo, fl; - MBEDTLS_MPS_TRACE_INIT( "reader_reclaim" ); + MBEDTLS_MPS_TRACE_INIT( "mbedtls_mps_reader_reclaim" ); if( paused != NULL ) *paused = 0; @@ -429,6 +390,7 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *rd, { MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, "No unsatisfied read-request has been logged." ); + /* Check if there's data left to be consumed. */ if( commit < fo || commit - fo < fl ) { @@ -437,16 +399,28 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *rd, rd->end = commit; MBEDTLS_MPS_TRACE_RETURN( MBEDTLS_ERR_MPS_READER_DATA_LEFT ); } + + rd->acc_avail = 0; + rd->acc_share.acc_remaining = 0; + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "The fragment has been completely processed and committed." ); + "Fragment has been fully processed and committed." ); } else { + int overflow; + + mbedtls_mps_size_t acc_backup_offset; + mbedtls_mps_size_t acc_backup_len; mbedtls_mps_size_t frag_backup_offset; mbedtls_mps_size_t frag_backup_len; + + mbedtls_mps_size_t backup_len; + mbedtls_mps_size_t acc_len_needed; + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "There has been an unsatisfied read-request with %u bytes overhead.", - (unsigned) pending ); + "There has been an unsatisfied read with %u bytes overhead.", + (unsigned) pending ); if( acc == NULL ) { @@ -462,69 +436,61 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *rd, if( commit < fo ) { /* No, accumulator is still being processed. */ - int overflow; - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "Still processing data from the accumulator" ); - - overflow = - ( fo + fl < fo ) || ( fo + fl + pending < fo + fl ); - if( overflow || al < fo + fl + pending ) - { - rd->end = commit; - rd->pending = 0; - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "The accumulator is too small to handle the backup." ); - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "* Remaining size: %u", (unsigned) al ); - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "* Needed: %u (%u + %u + %u)", - (unsigned) ( fo + fl + pending ), - (unsigned) fo, (unsigned) fl, (unsigned) pending ); - MBEDTLS_MPS_TRACE_RETURN( - MBEDTLS_ERR_MPS_READER_ACCUMULATOR_TOO_SMALL ); - } frag_backup_offset = 0; frag_backup_len = fl; + acc_backup_offset = commit; + acc_backup_len = fo - commit; } else { /* Yes, the accumulator is already processed. */ - int overflow; - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "The accumulator has already been processed" ); - - frag_backup_offset = commit; - frag_backup_len = fl - commit; - overflow = ( frag_backup_len + pending < pending ); - - if( overflow || - al - fo < frag_backup_len + pending ) - { - rd->end = commit; - rd->pending = 0; - - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "The accumulator is too small to handle the backup." ); - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "* Remaining size: %u", (unsigned) ( al - fo ) ); - MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, - "* Needed: %u (%u + %u)", - (unsigned) ( frag_backup_len + pending ), - (unsigned) frag_backup_len, (unsigned) pending ); - MBEDTLS_MPS_TRACE_RETURN( - MBEDTLS_ERR_MPS_READER_ACCUMULATOR_TOO_SMALL ); - } + frag_backup_offset = commit - fo; + frag_backup_len = fl - frag_backup_offset; + acc_backup_offset = 0; + acc_backup_len = 0; } - frag += frag_backup_offset; - acc += fo; - memcpy( acc, frag, frag_backup_len ); + backup_len = acc_backup_len + frag_backup_len; + acc_len_needed = backup_len + pending; + + overflow = 0; + overflow |= ( backup_len < acc_backup_len ); + overflow |= ( acc_len_needed < backup_len ); + + if( overflow || al < acc_len_needed ) + { + /* Except for the different return code, we behave as if + * there hadn't been a call to mbedtls_mps_reader_get() + * since the last commit. */ + rd->end = commit; + rd->pending = 0; + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, + "The accumulator is too small to handle the backup." ); + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, + "* Size: %u", (unsigned) al ); + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_error, + "* Needed: %u (%u + %u)", + (unsigned) acc_len_needed, + (unsigned) backup_len, (unsigned) pending ); + MBEDTLS_MPS_TRACE_RETURN( + MBEDTLS_ERR_MPS_READER_ACCUMULATOR_TOO_SMALL ); + } MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, - "Backup %u bytes into accumulator", - (unsigned) frag_backup_len ); + "Fragment backup: %u", (unsigned) frag_backup_len ); + MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, + "Accumulator backup: %u", (unsigned) acc_backup_len ); - rd->acc_avail = fo + frag_backup_len; + /* Move uncommitted parts from the accumulator to the front + * of the accumulator. */ + memmove( acc, acc + acc_backup_offset, acc_backup_len ); + + /* Copy uncmmitted parts of the current fragment to the + * accumulator. */ + memcpy( acc + acc_backup_len, + frag + frag_backup_offset, frag_backup_len ); + + rd->acc_avail = backup_len; rd->acc_share.acc_remaining = pending; if( paused != NULL ) @@ -534,14 +500,13 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *rd, rd->frag = NULL; rd->frag_len = 0; - rd->commit = 0; - rd->end = 0; - rd->pending = 0; + rd->commit = 0; + rd->end = 0; + rd->pending = 0; MBEDTLS_MPS_TRACE( mbedtls_mps_trace_comment, "Final state: aa %u, al %u, ar %u", (unsigned) rd->acc_avail, (unsigned) rd->acc_len, (unsigned) rd->acc_share.acc_remaining ); - MBEDTLS_MPS_TRACE_RETURN( 0 ); } diff --git a/library/mps_reader.h b/library/mps_reader.h index 5648ede83..560445732 100644 --- a/library/mps_reader.h +++ b/library/mps_reader.h @@ -31,7 +31,7 @@ * a 'consumer' which fetches and processes it in chunks of * again arbitrary, and potentially different, size. * - * Readers can be seen as datagram-to-stream converters, + * Readers can thus be seen as datagram-to-stream converters, * and they abstract away the following two tasks from the user: * 1. The pointer arithmetic of stepping through a producer- * provided chunk in smaller chunks. @@ -54,36 +54,39 @@ * be satisfiable. * - Repeat the above. * - * From the perspective of the consumer, the state of the - * reader is a potentially empty list of input buffers that - * the reader has provided to the consumer. - * New buffers can be requested through calls to mbedtls_mps_reader_get(), - * while previously obtained input buffers can be marked processed - * through calls to mbedtls_mps_reader_consume(), emptying the list of - * input buffers and invalidating them from the consumer's perspective. - * The consumer need not be aware of the distinction between consumer - * and producer mode, because he only interfaces with the reader - * when the latter is in consuming mode. + * The abstract states of the reader from the producer's and + * consumer's perspective are as follows: * - * From the perspective of the producer, the state of the reader - * is one of the following: - * - Attached: An incoming data buffer is currently - * being managed by the reader, and - * - Unset: No incoming data buffer is currently - * managed by the reader, and all previously - * handed incoming data buffers have been - * fully processed. - * - Accumulating: No incoming data buffer is currently - * managed by the reader, but some data - * from the previous incoming data buffer - * hasn't been processed yet and is internally - * held back. - * The Unset and Accumulating states belong to producing mode, - * while the Attached state belongs to consuming mode. + * - From the perspective of the consumer, the state of the + * reader consists of the following: + * - A byte stream representing (concatenation of) the data + * received through calls to mbedtls_mps_reader_get(), + * - A marker within that byte stream indicating which data + * need not be retained when the reader is passed back to + * the producer via mbedtls_mps_reader_reclaim(). + * The marker can be set via mbedtls_mps_reader_commit() + * which places it at the end of the current byte stream. + * The consumer need not be aware of the distinction between consumer + * and producer mode, because he only interfaces with the reader + * when the latter is in consuming mode. * - * Transitioning from Unset or Accumulating to Attached is - * done via calls to mbedtls_mps_reader_feed(), while transitioning - * from Consuming to either Unset or Accumulating (depending + * - From the perspective of the producer, the reader's state is one of: + * - Attached: The reader is in consuming mode. + * - Unset: No incoming data buffer is currently managed by the reader, + * and all previously handed incoming data buffers have been + * fully processed. More data needs to be fed into the reader + * via mbedtls_mps_reader_feed(). + * + * - Accumulating: No incoming data buffer is currently managed by the + * reader, but some data from the previous incoming data + * buffer hasn't been processed yet and is internally + * held back. + * The Attached state belongs to consuming mode, while the Unset and + * Accumulating states belong to producing mode. + * + * Transitioning from the Unset or Accumulating state to Attached is + * done via successful calls to mbedtls_mps_reader_feed(), while + * transitioning from Consuming to either Unset or Accumulating (depending * on what has been processed) is done via mbedtls_mps_reader_reclaim(). * * The following diagram depicts the producer-state progression: @@ -94,9 +97,9 @@ * | | | | * | | | | * | feed +---------+---+--+ | - * +--------------------------------------> Attached <---+ - * | / | - * +--------------------------------------> Consuming <---+ + * +--------------------------------------> <---+ + * | Attached | + * +--------------------------------------> <---+ * | feed, enough data available +---------+---+--+ | * | to serve previous consumer request | | | * | | | | @@ -108,6 +111,10 @@ * +--------+ * feed, need more data to serve * previous consumer request + * | + * | + * producing mode | consuming mode + * | * */ @@ -141,7 +148,7 @@ struct mbedtls_mps_reader /*!< The offset of the last commit, relative * to the first byte in the accumulator. * This is only used when the reader is in - * consuming mode, i.e. frag != NULL; + * consuming mode, i.e. \c frag != \c NULL; * otherwise, its value is \c 0. */ mbedtls_mps_stored_size_t end; /*!< The offset of the end of the last chunk @@ -306,8 +313,7 @@ int mbedtls_mps_reader_reclaim( mbedtls_mps_reader *reader, * address of a buffer of size \c *buflen * (if \c buflen != \c NULL) or \c desired * (if \c buflen == \c NULL). The user hass ownership - * of the buffer until the next call to mbedtls_mps_reader_commit(). - * or mbedtls_mps_reader_reclaim(). + * of the buffer until the next call mbedtls_mps_reader_reclaim(). * \return #MBEDTLS_ERR_MPS_READER_OUT_OF_DATA if there is not enough * data available to serve the read request. In this case, * the reader remains intact, and additional data can be @@ -329,19 +335,22 @@ int mbedtls_mps_reader_get( mbedtls_mps_reader *reader, mbedtls_mps_size_t *buflen ); /** - * \brief Signal that all input buffers previously obtained - * from mbedtls_writer_get() are fully processed. + * \brief Mark data obtained from mbedtls_writer_get() as processed. * - * This function marks the previously fetched data as fully - * processed and invalidates their respective buffers. + * This call indicates that all data received from prior calls to + * mbedtls_mps_reader_fetch() has been or will have been + * processed when mbedtls_mps_reader_reclaim() is called, + * and thus need not be backed up. * - * \param reader The reader context to use. + * This function has no user observable effect until + * mbedtls_mps_reader_reclaim() is called. In particular, + * buffers received from mbedtls_mps_reader_fetch() remain + * valid until mbedtls_mps_reader_reclaim() is called. * - * \return \c 0 on success. - * \return A negative \c MBEDTLS_ERR_READER_XXX error code on failure. + * \param reader The reader context to use. * - * \warning Once this function is called, you must not use the - * pointers corresponding to the committed data anymore. + * \return \c 0 on success. + * \return A negative \c MBEDTLS_ERR_READER_XXX error code on failure. * */ int mbedtls_mps_reader_commit( mbedtls_mps_reader *reader );