Skip to content

Commit c0ec8f3

Browse files
committed
Address review: narrow CryptoCb introspection to a boolean check
- Replace public wc_CryptoCb_GetDevice() with wc_CryptoCb_IsDeviceRegistered() returns 1 or 0. keep the CryptoCb struct and GetDevice private. - Reject RegisterDevice(INVALID_DEVID) with BAD_FUNC_ARG instead of ALREADY_E. - Document the new API and the ALREADY_E/BAD_FUNC_ARG returns. - Fix table-full test to not leak when MAX_CRYPTO_DEVID_CALLBACKS >= 256.
1 parent 451eaf9 commit c0ec8f3

4 files changed

Lines changed: 75 additions & 31 deletions

File tree

doc/dox_comments/header_files/cryptocb.h

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
1414
\return CRYPTOCB_UNAVAILABLE to fallback to using software crypto
1515
\return 0 for success
16+
\return ALREADY_E if devId is already registered. A devId must be
17+
un-registered with wc_CryptoCb_UnRegisterDevice before it can be
18+
registered again; re-registering an active devId is not an in-place update.
19+
\return BAD_FUNC_ARG if devId is INVALID_DEVID (-2)
1620
\return negative value for failure
1721
1822
\param devId any unique value, not -2 (INVALID_DEVID)
@@ -98,6 +102,32 @@
98102
*/
99103
int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx);
100104

105+
/*!
106+
\ingroup CryptoCb
107+
108+
\brief This function reports whether a crypto callback device identifier
109+
(devID) is currently registered. It is useful for checking registration
110+
state before calling wc_CryptoCb_RegisterDevice, which now rejects an
111+
already-registered devID with ALREADY_E.
112+
113+
\return 1 if the device ID is registered
114+
\return 0 if the device ID is not registered, or if devId is
115+
INVALID_DEVID (-2)
116+
117+
\param devId the device identifier to query
118+
119+
_Example_
120+
\code
121+
if (!wc_CryptoCb_IsDeviceRegistered(devId)) {
122+
wc_CryptoCb_RegisterDevice(devId, myCryptoCb_Func, &myCtx);
123+
}
124+
\endcode
125+
126+
\sa wc_CryptoCb_RegisterDevice
127+
\sa wc_CryptoCb_UnRegisterDevice
128+
*/
129+
int wc_CryptoCb_IsDeviceRegistered(int devId);
130+
101131
/*!
102132
\ingroup CryptoCb
103133

tests/api.c

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29228,28 +29228,27 @@ static int test_wc_CryptoCb(void)
2922829228
int tlsVer;
2922929229
#endif
2923029230
/* Exercise the exposed CryptoCb device-management API:
29231-
* wc_CryptoCb_RegisterDevice / UnRegisterDevice / GetDevice /
29231+
* wc_CryptoCb_RegisterDevice / UnRegisterDevice / IsDeviceRegistered /
2923229232
* DefaultDevID (and InfoString when DEBUG_CRYPTOCB is enabled). */
2923329233
{
2923429234
int getDevId = 1234;
2923529235
int getDevCtx = 0;
29236-
CryptoCb* dev = NULL;
2923729236
int i, n, rc;
2923829237

29239-
/* Unregistered devId is not found. */
29240-
ExpectNull(wc_CryptoCb_GetDevice(getDevId));
29238+
/* Unregistered devId is not reported as registered. */
29239+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(getDevId), 0);
2924129240

29242-
/* After registering, the device is found with matching fields. */
29241+
/* Registering with INVALID_DEVID is rejected. */
29242+
ExpectIntEQ(wc_CryptoCb_RegisterDevice(INVALID_DEVID, NULL, &getDevCtx),
29243+
WC_NO_ERR_TRACE(BAD_FUNC_ARG));
29244+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(INVALID_DEVID), 0);
29245+
29246+
/* After registering, the device is reported registered. */
2924329247
ExpectIntEQ(wc_CryptoCb_RegisterDevice(getDevId, NULL, &getDevCtx), 0);
29244-
ExpectNotNull(dev = wc_CryptoCb_GetDevice(getDevId));
29245-
if (dev != NULL) {
29246-
ExpectIntEQ(dev->devId, getDevId);
29247-
ExpectNull(dev->cb);
29248-
ExpectPtrEq(dev->ctx, &getDevCtx);
29249-
}
29248+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(getDevId), 1);
2925029249

29251-
/* A different, unregistered devId is still not found. */
29252-
ExpectNull(wc_CryptoCb_GetDevice(getDevId + 1));
29250+
/* A different, unregistered devId is still not reported registered. */
29251+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(getDevId + 1), 0);
2925329252

2925429253
/* Re-registering an already-registered devId is rejected. */
2925529254
ExpectIntEQ(wc_CryptoCb_RegisterDevice(getDevId, NULL, &getDevCtx),
@@ -29264,25 +29263,26 @@ static int test_wc_CryptoCb(void)
2926429263
ExpectIntNE(wc_CryptoCb_DefaultDevID(), INVALID_DEVID);
2926529264
#endif
2926629265

29267-
/* After unregistering, the device is no longer found. */
29266+
/* After unregistering, the device is no longer registered. */
2926829267
wc_CryptoCb_UnRegisterDevice(getDevId);
29269-
ExpectNull(wc_CryptoCb_GetDevice(getDevId));
29268+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(getDevId), 0);
2927029269

2927129270
/* Unregistering is a harmless no-op for unknown or invalid devIds. */
2927229271
wc_CryptoCb_UnRegisterDevice(getDevId); /* already removed */
2927329272
wc_CryptoCb_UnRegisterDevice(INVALID_DEVID); /* never a real devId */
29274-
ExpectNull(wc_CryptoCb_GetDevice(getDevId));
29273+
ExpectIntEQ(wc_CryptoCb_IsDeviceRegistered(getDevId), 0);
2927529274

2927629275
/* The device table is finite: once full, registration returns
29277-
* BUFFER_E. Register unique devIds until that happens, then clean up.
29278-
* The cap (256) just guards against an unexpectedly large table. */
29276+
* BUFFER_E. Registering MAX_CRYPTO_DEVID_CALLBACKS + 1 unique devIds is
29277+
* guaranteed to fill the table and hit BUFFER_E regardless of how many
29278+
* slots were already in use. Then unregister every id we tried
29279+
* (UnRegister is a no-op for the final failed attempt). */
2927929280
rc = 0;
29280-
for (i = 0; rc == 0 && i < 256; i++) {
29281+
for (i = 0; rc == 0 && i <= MAX_CRYPTO_DEVID_CALLBACKS; i++) {
2928129282
rc = wc_CryptoCb_RegisterDevice(0x5000 + i, NULL, NULL);
2928229283
}
2928329284
ExpectIntEQ(rc, WC_NO_ERR_TRACE(BUFFER_E));
29284-
/* Every id except the final failed attempt registered; unregister them. */
29285-
for (n = 0; n + 1 < i; n++) {
29285+
for (n = 0; n < i; n++) {
2928629286
wc_CryptoCb_UnRegisterDevice(0x5000 + n);
2928729287
}
2928829288

wolfcrypt/src/cryptocb.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ Crypto Callback Build Options:
8080
#include <wolfssl/wolfcrypt/port/caam/wolfcaam.h>
8181
#endif
8282
/* TODO: Consider linked list with mutex */
83-
#ifndef MAX_CRYPTO_DEVID_CALLBACKS
84-
#define MAX_CRYPTO_DEVID_CALLBACKS 8
85-
#endif
8683

84+
typedef struct CryptoCb {
85+
int devId;
86+
CryptoDevCallbackFunc cb;
87+
void* ctx;
88+
} CryptoCb;
8789
static WC_THREADSHARED CryptoCb gCryptoDev[MAX_CRYPTO_DEVID_CALLBACKS];
8890

8991
#ifdef WOLF_CRYPTO_CB_FIND
@@ -350,7 +352,7 @@ void wc_CryptoCb_InfoString(wc_CryptoInfo* info)
350352

351353
/* Search through listed devices and return the first matching device ID
352354
* found. */
353-
CryptoCb* wc_CryptoCb_GetDevice(int devId)
355+
static CryptoCb* wc_CryptoCb_GetDevice(int devId)
354356
{
355357
int i;
356358
for (i = 0; i < MAX_CRYPTO_DEVID_CALLBACKS; i++) {
@@ -360,6 +362,15 @@ CryptoCb* wc_CryptoCb_GetDevice(int devId)
360362
return NULL;
361363
}
362364

365+
/* Returns 1 if the given device ID is currently registered, 0 otherwise.
366+
* INVALID_DEVID marks free table slots, so it is never reported registered. */
367+
int wc_CryptoCb_IsDeviceRegistered(int devId)
368+
{
369+
if (devId == INVALID_DEVID)
370+
return 0;
371+
return wc_CryptoCb_GetDevice(devId) != NULL;
372+
}
373+
363374

364375
/* Filters through find callback set when trying to get the device,
365376
* returns the device found on success and null if not found. */
@@ -448,6 +459,10 @@ int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx)
448459
int rc = 0;
449460
CryptoCb* dev;
450461

462+
/* INVALID_DEVID marks a free slot and cannot be registered as a device. */
463+
if (devId == INVALID_DEVID)
464+
return BAD_FUNC_ARG;
465+
451466
/* Reject re-registration of an already-registered device ID. */
452467
if (wc_CryptoCb_GetDevice(devId) != NULL)
453468
return ALREADY_E;

wolfssl/wolfcrypt/cryptocb.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -672,19 +672,18 @@ typedef struct wc_CryptoInfo {
672672

673673
typedef int (*CryptoDevCallbackFunc)(int devId, struct wc_CryptoInfo* info, void* ctx);
674674

675-
typedef struct CryptoCb {
676-
int devId;
677-
CryptoDevCallbackFunc cb;
678-
void* ctx;
679-
} CryptoCb;
675+
/* Maximum number of crypto callback devices that can be registered. */
676+
#ifndef MAX_CRYPTO_DEVID_CALLBACKS
677+
#define MAX_CRYPTO_DEVID_CALLBACKS 8
678+
#endif
680679

681680
WOLFSSL_LOCAL void wc_CryptoCb_Init(void);
682681
WOLFSSL_LOCAL void wc_CryptoCb_Cleanup(void);
683682
WOLFSSL_LOCAL int wc_CryptoCb_GetDevIdAtIndex(int startIdx);
684683
WOLFSSL_API int wc_CryptoCb_RegisterDevice(int devId, CryptoDevCallbackFunc cb, void* ctx);
685684
WOLFSSL_API void wc_CryptoCb_UnRegisterDevice(int devId);
686685
WOLFSSL_API int wc_CryptoCb_DefaultDevID(void);
687-
WOLFSSL_API CryptoCb* wc_CryptoCb_GetDevice(int devId);
686+
WOLFSSL_API int wc_CryptoCb_IsDeviceRegistered(int devId);
688687

689688
#ifdef WOLF_CRYPTO_CB_FIND
690689
typedef int (*CryptoDevCallbackFind)(int devId, int algoType);

0 commit comments

Comments
 (0)