msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Spec

msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Specifically, this does away with pldm_msgbuf_init_cc() and all the
associated pldm_msgbuf_status() error translation machinery. All the
call-sites are fixed up, with some additional safety checks put in place
along the way.

I believe this change is viable because unless we're converting legacy
API implementations to use msgbuf there's no additional trickery, and
if we're converting existing implementations then care is required
regardless. The change of approach has no impact on implementation of
new APIs with msgbuf, as the current philosophy is that they should
return negative errnos anyway.

As seems to be the case with this kind of work, the parameter register
allocation seems to have been affected for a number of library APIs.
These are listed in the changelog, and the ABI dump has been updated.

Finally, for msgbuf use in the test cases, all instances have
been converted to use errnos in place of PLDM completion codes in the
expectations. Hopefully there's no more malarky with PLDM completion
code misuse in the future.

Change-Id: Id4a7366ee9f60fb991dfe84aa0bb5aadc9855fcc
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Improve type-specific ergonomics to match generic macros

Don't require that a pointer be passed. Rather, take the pointer inside
the type-safe macro definition, and perform the void cast to

msgbuf: Improve type-specific ergonomics to match generic macros

Don't require that a pointer be passed. Rather, take the pointer inside
the type-safe macro definition, and perform the void cast to avoid the
alignment warning.

Change-Id: I5fbfc4a95591d2640595107e6f5fcae44a95950f
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Allow pldm_msgbuf_span_required to accept NULL

Allow pldm_msgbuf_span_required to accept NULL as an argument
so we can use this API to skip past data in the msg buffer which
is not required

msgbuf: Allow pldm_msgbuf_span_required to accept NULL

Allow pldm_msgbuf_span_required to accept NULL as an argument
so we can use this API to skip past data in the msg buffer which
is not required and extract only the relevant data.

Change-Id: I08d233b8efe415732fb7c01c00a9925f04666fe2
Signed-off-by: Varsha Kaverappa <vkaverap@in.ibm.com>

show more ...

compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}

This allows us to elide checks where they're not necessary, and warn
people at compile-time when they're doing things they shouldn't.

Note that this c

compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}

This allows us to elide checks where they're not necessary, and warn
people at compile-time when they're doing things they shouldn't.

Note that this comes with an apparent ABI break. abi-compliance-checker
reports:

```
platform.h, libpldm.so.0.8.0
[−] decode_sensor_op_data ( uint8_t const* sensor_data, size_t sensor_data_length, uint8_t* present_op_state, uint8_t* previous_op_state )
Change: The parameter previous_op_state became passed in r8 register instead of rcx.
Effect Applications will read the wrong memory block instead of the parameter value.
```

It's unclear to me why. The signature hasn't changed, but how the
implementation tests the parameter values has.

Change-Id: Ie8d8bc1641280522532d9b4764bf07c64b1921c8
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Harden pldm_msgbuf_{insert,extract}_array()

Review of some proposed APIs suggested that correct use of the
pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it
should be.

msgbuf: Harden pldm_msgbuf_{insert,extract}_array()

Review of some proposed APIs suggested that correct use of the
pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it
should be. In the three-parameter form, it was too tempting to provide
the length to extract as parsed out of a PLDM message. The intended
use was that the length parameter represented the length of the
user-provided data buffer.

Instead, move to a four-parameter form, provide reasonable documentation
for how these APIs should be used, fix all the call-sites, and deprecate
some existing unsafe APIs.

Change-Id: If58e5574600e80b354f383554283c4eda5d7234c
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add pldm_msgbuf_copy_string_utf16()

Safely copy a NUL-terminated UTF16-{BE,LE} string between msgbuf
instances.

Change-Id: If96df9598f17ac771d75f0831be270c5e0139578
Signed-off-by: Andrew Je

msgbuf: Add pldm_msgbuf_copy_string_utf16()

Safely copy a NUL-terminated UTF16-{BE,LE} string between msgbuf
instances.

Change-Id: If96df9598f17ac771d75f0831be270c5e0139578
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add pldm_msgbuf_span_string_utf16()

Add pldm_msgbuf_span_string_utf16 API to return the start pointer of the
utf16 string in message buffer. The API also returns the UTF16 string
length in t

msgbuf: Add pldm_msgbuf_span_string_utf16()

Add pldm_msgbuf_span_string_utf16 API to return the start pointer of the
utf16 string in message buffer. The API also returns the UTF16 string
length in terms of bytes, including the NUL terminator.

```
__attribute__((always_inline)) static inline int
pldm_msgbuf_span_string_utf16(struct pldm_msgbuf *ctx, void **cursor,
size_t *length)
```

The `cursor` and `length` are optional. Input NULL to `cursor` and
`length` will cause the message buffer cursor points to remaining data.
The caller can ignore `length` option by input NULL if they don't care
about the size of utf16 string.

Change-Id: I1fc2865a21d9925e49416531b85212b3b07dc37a
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add pldm_msgbuf_copy_string_ascii()

Safely copy a NUL-terminated string between msgbuf instances.

Change-Id: I224dc3f5bbd55fd9d4727ab0de065d5253ee0bea
Signed-off-by: Andrew Jeffery <andrew@

msgbuf: Add pldm_msgbuf_copy_string_ascii()

Safely copy a NUL-terminated string between msgbuf instances.

Change-Id: I224dc3f5bbd55fd9d4727ab0de065d5253ee0bea
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add pldm_msgbuf_span_string_ascii()

Add pldm_msgbuf_span_string_ascii() API to find the start of the ascii
string in the message buffer.

```
pldm_msgbuf_span_string_ascii(struct pldm_msgbuf

msgbuf: Add pldm_msgbuf_span_string_ascii()

Add pldm_msgbuf_span_string_ascii() API to find the start of the ascii
string in the message buffer.

```
pldm_msgbuf_span_string_ascii(struct pldm_msgbuf *ctx, void **cursor,
size_t *length)
```

The API returns the start pointer of ascii string in the message buffer
and length of that ascii string includes Terminator.
The `cursor` and `length` are optional. Input NULL to `cursor` and
`length` will cause the message buffer cursor points to remaining data.
The caller can ignore `length` option by input NULL if they don't care
about the size of ascii string.

Change-Id: I4a73b7425ee1e4e5621eb16de6e16189efdf202b
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Generalize array extraction and insertion

Build the type-safe and generic behavior on top of memcpy() via a
"private" helper that takes a void pointer.

Change-Id: Iedb8e9237c780735d4cac41fe

msgbuf: Generalize array extraction and insertion

Build the type-safe and generic behavior on top of memcpy() via a
"private" helper that takes a void pointer.

Change-Id: Iedb8e9237c780735d4cac41fe0a723c3751c64ce
Signed-off-by: Chris Wang <chris.wang.wiwynn@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add error code personalities

libpldm is in a bit of a transitional period with respect to returned
error codes. A historical choice was to return PLDM completion codes
from the library API t

msgbuf: Add error code personalities

libpldm is in a bit of a transitional period with respect to returned
error codes. A historical choice was to return PLDM completion codes
from the library API to indicate errors. This is unfortunate because
we're now constrained to errors that are specified by the PLDM protocol,
which is much less expressive than the set of errors that might be
produced by a run-time environment for the library.

The choice going forward is to return C's errno codes. However at this
point we step on another rake in the libpldm design, which is that some
internal data structures are very much the wire format of corresponding
PLDM messages (such as the PDR repository implementation). Working with
wire-format buffers is most safely done via the msgbuf APIs, however we
then hit the conflict of different error code styles in various parts of
the API surface.

Do a bit of surgery to provide different error code personalities for
msgbuf, such that the caller can pick the style of error code they need
it to return to maintain consistency.

Note that like the previous patch marking all msgbuf APIs as
__attribute__((always_inline)), the rework here makes another small
impact on the argument register allocation of several stable APIs. The
ABI dump is updated accordingly.

Change-Id: Id59c39c5c822f514f546dab88575317071a97c96
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add copy API

pldm_msgbuf_copy API allows copy of data from one msg buffer
to another. This was done earlier with a pldm_msgbuf_extract()
followed by pldm_msgbuf_insert().

Change-Id: I159792

msgbuf: Add copy API

pldm_msgbuf_copy API allows copy of data from one msg buffer
to another. This was done earlier with a pldm_msgbuf_extract()
followed by pldm_msgbuf_insert().

Change-Id: I159792f726916761894aefb0a8795f1f0dc84114
Signed-off-by: Varsha Kaverappa <vkaverap@in.ibm.com>

show more ...

msgbuf: Remove use of ssize_t for overflow tracking

There are a few concerns with the use of ssize_t in this context:

1. It's defined by POSIX and not C, and I'd prefer we not require POSIX
conc

msgbuf: Remove use of ssize_t for overflow tracking

There are a few concerns with the use of ssize_t in this context:

1. It's defined by POSIX and not C, and I'd prefer we not require POSIX
concepts where we can avoid it
2. ssize_t is defined over [-1, SSIZE_MAX] - it is not defined to have
the range of a regular signed type.

The source of both these statements is The Open Group Base
Specifications Issue 7, 2018 edition. IEEE Std 1003.1-2017 (Revision of
IEEE Std 1003.1-2008)

The second point directly contradicts how I was trying to use ssize_t in
the msgbuf implementation. As a result, switch the type of `remaining`
to intmax_t. Usually intmax_t is a problem child, but it's not used in
any public API, and it has the semantics I wanted by contrast to the
definition of ssize_t.

Note that we add assert() calls where we know the value of remaining
must be negative. Without the addition of the `assert()` calls in the
underflow checks, clang-analyzer gets tripped up by not being able to
prove `INTMAX_MIN + (intmax_t)sizeof(uint16_t) < 0`:

```
../src/platform.c:17:18: error: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
17 | if (ctx->length + sizeof(*ctx) < lower) {
| ^
../src/platform.c:2445:6: note: 'rc' is 0
2445 | if (rc) {
| ^~
../src/platform.c:2445:2: note: Taking false branch
2445 | if (rc) {
| ^
../src/platform.c:2449:7: note: Calling 'pldm_msgbuf_extract_value_pdr_hdr'
2449 | rc = pldm_msgbuf_extract_value_pdr_hdr(buf, &hdr);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/msgbuf/platform.h:17:2: note: Calling 'pldm__msgbuf_extract_uint16'
17 | pldm_msgbuf_extract(ctx, hdr->length);
| ^
../src/msgbuf/../msgbuf.h:517:2: note: expanded from macro 'pldm_msgbuf_extract'
517 | _Generic((dst), \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
518 | uint8_t: pldm__msgbuf_extract_uint8, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
519 | int8_t: pldm__msgbuf_extract_int8, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
520 | uint16_t: pldm__msgbuf_extract_uint16, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
521 | int16_t: pldm__msgbuf_extract_int16, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
522 | uint32_t: pldm__msgbuf_extract_uint32, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
523 | int32_t: pldm__msgbuf_extract_int32, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
524 | real32_t: pldm__msgbuf_extract_real32)(ctx, (void *)&(dst))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/msgbuf/../msgbuf.h:341:7: note: 'ctx' is non-null
341 | if (!ctx || !ctx->cursor || !dst) {
| ^~~
../src/msgbuf/../msgbuf.h:341:6: note: Left side of '||' is false
341 | if (!ctx || !ctx->cursor || !dst) {
| ^
../src/msgbuf/../msgbuf.h:341:20: note: Field 'cursor' is non-null
341 | if (!ctx || !ctx->cursor || !dst) {
| ^
../src/msgbuf/../msgbuf.h:341:6: note: Left side of '||' is false
341 | if (!ctx || !ctx->cursor || !dst) {
| ^
../src/msgbuf/../msgbuf.h:341:31: note: 'dst' is non-null
341 | if (!ctx || !ctx->cursor || !dst) {
| ^~~
../src/msgbuf/../msgbuf.h:341:2: note: Taking false branch
341 | if (!ctx || !ctx->cursor || !dst) {
| ^
../src/msgbuf/../msgbuf.h:347:2: note: Taking true branch
347 | if (ctx->remaining < INTMAX_MIN + (intmax_t)sizeof(ldst)) {
| ^
../src/msgbuf/../msgbuf.h:348:3: note: Returning without writing to '*dst'
348 | return PLDM_ERROR_INVALID_LENGTH;
| ^
../src/msgbuf/platform.h:17:2: note: Returning from 'pldm__msgbuf_extract_uint16'
17 | pldm_msgbuf_extract(ctx, hdr->length);
| ^
../src/msgbuf/../msgbuf.h:517:2: note: expanded from macro 'pldm_msgbuf_extract'
517 | _Generic((dst), \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
518 | uint8_t: pldm__msgbuf_extract_uint8, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
519 | int8_t: pldm__msgbuf_extract_int8, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
520 | uint16_t: pldm__msgbuf_extract_uint16, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
521 | int16_t: pldm__msgbuf_extract_int16, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
522 | uint32_t: pldm__msgbuf_extract_uint32, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
523 | int32_t: pldm__msgbuf_extract_int32, \
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
524 | real32_t: pldm__msgbuf_extract_real32)(ctx, (void *)&(dst))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/msgbuf/platform.h:19:2: note: Returning without writing to 'hdr->length'
19 | return pldm_msgbuf_validate(ctx);
| ^
../src/platform.c:2449:7: note: Returning from 'pldm_msgbuf_extract_value_pdr_hdr'
2449 | rc = pldm_msgbuf_extract_value_pdr_hdr(buf, &hdr);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/platform.c:2450:6: note: 'rc' is 0
2450 | if (rc) {
| ^~
../src/platform.c:2450:2: note: Taking false branch
2450 | if (rc) {
| ^
../src/platform.c:2454:7: note: Calling 'pldm_platform_pdr_hdr_validate'
2454 | rc = pldm_platform_pdr_hdr_validate(
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2455 | &hdr, PLDM_PDR_NUMERIC_EFFECTER_PDR_MIN_LENGTH,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2456 | pdr_data_length);
| ~~~~~~~~~~~~~~~~
../src/platform.c:17:18: note: The left operand of '+' is a garbage value
17 | if (ctx->length + sizeof(*ctx) < lower) {
| ~~~~~~~~~~~ ^
```

Change-Id: Idbe5a14455ad677a39c8f535eddd9c2ce471c783
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Rework detection of invalid memory regions

From Annex J.2 of N2176 (C17 draft specification):

> Addition or subtraction of a pointer into, or just beyond, an array
> object and an integer t

msgbuf: Rework detection of invalid memory regions

From Annex J.2 of N2176 (C17 draft specification):

> Addition or subtraction of a pointer into, or just beyond, an array
> object and an integer type produces a result that does not point into,
> or just beyond, the same array object (6.5.6).

Instead we can lean on uintptr_t from 7.20.1.4, and from there the
defined behavior of unsigned overflow.

Change-Id: Ia1b47b87efeb9c96057d294a3e38e90bfdba5386
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Add insert and span APIs

pldm_msgbuf_insert() is generic APIs to insert data to a pointer.
It automatically calls the correct insertor API for the type of the
packing data.
pldm_msgbuf_span_

msgbuf: Add insert and span APIs

pldm_msgbuf_insert() is generic APIs to insert data to a pointer.
It automatically calls the correct insertor API for the type of the
packing data.
pldm_msgbuf_span_required() spans the input number of bytes from
buffer to the input double pointer.
pldm_msgbuf_span_remaining() extracts the remaining number of bytes
from buffer to the input double pointer.

Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Change-Id: Ide7dbf735f69a8288e6f39a0b7b5c33aad38a98e

show more ...

msgbuf: Add pldm_msgbuf_consumed()

pldm_msgbuf_consumed() only returns PLDM_SUCCESS if a message buffer has
been exactly exhausted - all bytes read and no overflow has occurred.

The decode_* functi

msgbuf: Add pldm_msgbuf_consumed()

pldm_msgbuf_consumed() only returns PLDM_SUCCESS if a message buffer has
been exactly exhausted - all bytes read and no overflow has occurred.

The decode_* functions have a mix of behaviour, some lenient like the
behaviour provided by pldm_msgbuf_validate() and others strict like
pldm_msgbuf_consumed().

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: If058ed3748dcc7cb366f4c31344c9bbfba1c5939

show more ...

msgbuf: Add pldm_msgbuf_extract_array() for uint8

This is required for converting the
decode_get_pdr_repository_info_resp() function to pldm_msgbuf.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

msgbuf: Add pldm_msgbuf_extract_array() for uint8

This is required for converting the
decode_get_pdr_repository_info_resp() function to pldm_msgbuf.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: If9afcaa83872969bcf8e4d0ecdeae2971e12248b

show more ...

Introduce a small msgbuf abstraction

Tidy up how we extract data from wire-format buffers.

The API achieves the following:

1. Abstracts the buffer tracking to improve clarity in the calling code

Introduce a small msgbuf abstraction

Tidy up how we extract data from wire-format buffers.

The API achieves the following:

1. Abstracts the buffer tracking to improve clarity in the calling code

2. Prevents buffer overflows during data extraction

3. Handles type conversions while avoiding undefined behaviour

4. Handles alignment concerns with packed data and removes the need for
`__attribute__((packed))` structs in the public ABI

5. Handles the endianness conversions required by the PLDM
specification(s)

6. Batches error checks such that you mostly only have to do `return
pldm_msgbuf_destroy();` at the end of the decode_* function for error
handling, no error handling required on every `pldm_msgbuf_extract()`
call

7. pldm_msgbuf_extract() is generic over the type of the data pointer
(automatically calls the correct extractor for the type of the
pointer)

8. Generates optimal code (where the optimiser can prove the accesses
are in-bounds we generate straight-line load/store pairs and no
function calls)

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I7e727cbd26c43aae2815ababe0e6ca4c8e629766
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...