Skip to content

storage: port host-flash device to the Phoenix-RTOS platform #574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
agkaminski wants to merge 1 commit into
base: master
Choose a base branch
from
Open

storage: port host-flash device to the Phoenix-RTOS platform #574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion _targets/Makefile.ia32-generic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
# Copyright 2019 Phoenix Systems
#

DEFAULT_COMPONENTS := pc-tty uart16550 pc-ata libusbehci umass libusbdrv-umass
DEFAULT_COMPONENTS := pc-tty uart16550 pc-ata libusbehci umass libusbdrv-umass host-flash
108 changes: 58 additions & 50 deletions storage/host-flash/host-flash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <assert.h>

#include "host-flash.h"

Expand All @@ -33,76 +34,96 @@ static struct {
} hostflash_common;


ssize_t hostflash_read(struct _meterfs_devCtx_t *devCtx, off_t offs, void *buff, size_t bufflen)
static ssize_t hostflash_safeRead(void *buff, size_t bufflen, off_t offset)
{
ssize_t stat;
int ret = lseek(hostflash_common.filefd, offset, SEEK_SET);
Copy link

@gemini-code-assist gemini-code-assist bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The return type of lseek is off_t, but it's being stored in an int. This could lead to truncation on 64-bit systems where off_t is larger than int. Please use off_t for the ret variable to ensure correctness across different architectures.

Suggested change
int ret = lseek(hostflash_common.filefd, offset, SEEK_SET);
off_t ret = lseek(hostflash_common.filefd, offset, SEEK_SET);

if (ret < 0) {
return (ssize_t)ret;
}

int readsz = 0;
Copy link

@gemini-code-assist gemini-code-assist bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The readsz variable is of type int, but it accumulates the number of bytes read into a buffer whose size is given by bufflen (a size_t). If bufflen is larger than INT_MAX, readsz could overflow, leading to unpredictable behavior. It is safer to declare readsz as size_t to prevent this.

Suggested change
int readsz = 0;
size_t readsz = 0;

ssize_t len;
do {
len = read(hostflash_common.filefd, (char *)buff + readsz, bufflen - readsz);
if (len < 0) {
if (errno == EINTR) {
continue;
}
return len;
}
readsz += len;
} while ((len > 0) && (readsz != bufflen));

(void)devCtx;
return readsz;
}

if ((devCtx == NULL) || (devCtx->state == 0) || ((offs + bufflen) > hostflash_common.flashsz)) {
return -EINVAL;

static ssize_t hostflash_safeWrite(const void *buff, size_t bufflen, off_t offset)
{
int ret = lseek(hostflash_common.filefd, offset, SEEK_SET);
Copy link

@gemini-code-assist gemini-code-assist bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The return type of lseek is off_t, but it's being stored in an int. This could lead to truncation on 64-bit systems where off_t is larger than int. Please use off_t for the ret variable to ensure correctness across different architectures.

Suggested change
int ret = lseek(hostflash_common.filefd, offset, SEEK_SET);
off_t ret = lseek(hostflash_common.filefd, offset, SEEK_SET);

if (ret < 0) {
return (ssize_t)ret;
}

while ((stat = pread(hostflash_common.filefd, (void *)((char *)buff + readsz), bufflen - readsz, offs + readsz)) != 0) {
if (stat < 0) {
int writesz = 0;
Copy link

@gemini-code-assist gemini-code-assist bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The writesz variable is of type int, but it accumulates the number of bytes written from a buffer whose size is given by bufflen (a size_t). If bufflen is larger than INT_MAX, writesz could overflow, leading to unpredictable behavior. It is safer to declare writesz as size_t to prevent this.

Suggested change
int writesz = 0;
size_t writesz = 0;

ssize_t len;
do {
len = write(hostflash_common.filefd, (char *)buff + writesz, bufflen - writesz);
if (len < 0) {
if (errno == EINTR) {
continue;
}

return stat;
return len;
}
writesz += len;
} while ((len > 0) && (writesz != bufflen));

readsz += stat;
if (readsz == bufflen) {
break;
}
return writesz;
}


ssize_t hostflash_read(struct _meterfs_devCtx_t *devCtx, off_t offs, void *buff, size_t bufflen)
{
if ((devCtx == NULL) || (devCtx->state == 0) || ((offs + bufflen) > hostflash_common.flashsz)) {
return -EINVAL;
}

return readsz;
return hostflash_safeRead(buff, bufflen, offs);
}


ssize_t hostflash_write(struct _meterfs_devCtx_t *devCtx, off_t offs, const void *buff, size_t bufflen)
{
char tempTab[256];
size_t wrote = 0, i;
int len;
ssize_t stat, readsz;
size_t wrote = 0;

if ((devCtx == NULL) || (devCtx->state == 0) || ((offs + bufflen) > hostflash_common.flashsz)) {
return -EINVAL;
}

while (wrote != bufflen) {
size_t len;
if ((bufflen - wrote) >= sizeof(tempTab)) {
len = sizeof(tempTab);
}
else {
len = bufflen - wrote;
}

readsz = hostflash_read(devCtx, offs + wrote, tempTab, len);
ssize_t readsz = hostflash_safeRead(tempTab, len, offs + wrote);
if (readsz <= 0) {
return readsz;
}

for (i = 0; i < readsz; i++) {
tempTab[i] = tempTab[i] & *((const char *)buff + wrote + i);
for (ssize_t i = 0; i < readsz; i++) {
tempTab[i] &= *((const char *)buff + wrote + i);
}

len = 0;
while (len != readsz) {
stat = pwrite(hostflash_common.filefd, &tempTab, readsz - len, offs + wrote + len);
if (stat < 0) {
if (errno == EINTR)
continue;

return stat;
}
len += stat;
ssize_t stat = hostflash_safeWrite(tempTab, readsz, offs + wrote);
if (stat < 0) {
return stat;
}
wrote += len;
wrote += stat;
}

return (ssize_t)wrote;
Expand All @@ -112,35 +133,22 @@ ssize_t hostflash_write(struct _meterfs_devCtx_t *devCtx, off_t offs, const void
int hostflash_sectorErase(struct _meterfs_devCtx_t *devCtx, off_t offs)
{
char tempTab[256];
ssize_t len = sizeof(tempTab);
size_t erased = 0;
unsigned int sectorAddr;
int stat;

if ((devCtx == NULL) || (devCtx->state == 0) || (offs >= hostflash_common.flashsz)) {
return -EINVAL;
}

(void)memset(tempTab, 0xff, sizeof(tempTab));

sectorAddr = (offs / hostflash_common.sectorsz) * hostflash_common.sectorsz;
off_t sectorAddr = (offs / hostflash_common.sectorsz) * hostflash_common.sectorsz;

while (erased != hostflash_common.sectorsz) {
stat = pwrite(hostflash_common.filefd, tempTab, len, sectorAddr + erased);
if (stat < 0) {
if (errno == EINTR) {
continue;
}

return stat;
}
assert(hostflash_common.sectorsz % sizeof(tempTab) == 0);

len -= stat;
if (len == 0) {
len = sizeof(tempTab);
for (size_t erased = 0; erased < hostflash_common.sectorsz; erased += sizeof(tempTab)) {
ssize_t stat = hostflash_safeWrite(tempTab, sizeof(tempTab), sectorAddr + erased);
if (stat < 0) {
return (int)stat;
}
Comment on lines +148 to 151
Copy link

@gemini-code-assist gemini-code-assist bot Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This implementation does not handle short writes from hostflash_safeWrite. If hostflash_safeWrite writes fewer bytes than requested without returning an error (e.g., if write() returns 0 because the device is full), stat will be positive but less than sizeof(tempTab). This will result in a partially erased sector, which can lead to data corruption. The return value should be checked to ensure the entire buffer was written.

Suggested change
ssize_t stat = hostflash_safeWrite(tempTab, sizeof(tempTab), sectorAddr + erased);
if (stat < 0) {
return (int)stat;
}
ssize_t stat = hostflash_safeWrite(tempTab, sizeof(tempTab), sectorAddr + erased);
if (stat != sizeof(tempTab)) {
return (stat < 0) ? (int)stat : -EIO;
}


erased += stat;
}

return 0;
Expand Down
Loading