Skip to content
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.

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

openamp: replace all strncpy to strlcpy #602

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions apps/system/linux/machine/generic/platform_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ static int sk_unix_client(const char *descr)

memset(&addr, 0, sizeof addr);
addr.sun_family = AF_UNIX;
strncpy(addr.sun_path, descr + strlen(UNIX_PREFIX),
strlcpy(addr.sun_path, descr + strlen(UNIX_PREFIX),
sizeof addr.sun_path);
if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) >= 0) {
printf("connected to %s\r\n", descr + strlen(UNIX_PREFIX));
Expand All @@ -178,7 +178,7 @@ static int sk_unix_server(const char *descr)
fd = socket(AF_UNIX, SOCK_STREAM, 0);

addr.sun_family = AF_UNIX;
strncpy(addr.sun_path, descr + strlen(UNIXS_PREFIX),
strlcpy(addr.sun_path, descr + strlen(UNIXS_PREFIX),
sizeof addr.sun_path);
unlink(addr.sun_path);
if (bind(fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
Expand Down
5 changes: 5 additions & 0 deletions lib/include/openamp/rpmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ typedef void (*rpmsg_ept_release_cb)(struct rpmsg_endpoint *ept);
typedef void (*rpmsg_ns_unbind_cb)(struct rpmsg_endpoint *ept);
typedef void (*rpmsg_ns_bind_cb)(struct rpmsg_device *rdev,
const char *name, uint32_t dest);
typedef int (*rpmsg_notify_wait_cb)(struct rpmsg_device *rdev, uint32_t id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please introduce this commit in a new PR, you also need to provide more information to explain why you need this update.


/**
* @brief Structure that binds a local RPMsg address to its user
Expand Down Expand Up @@ -150,7 +151,11 @@ struct rpmsg_device {
/** Callback handler for name service announcement, called when remote ept is destroyed */
rpmsg_ns_bind_cb ns_unbind_cb;

/** callback handler for rpmsg service, called when service can't get tx buffer */
rpmsg_notify_wait_cb notify_wait_cb;

/** RPMsg device operations */

struct rpmsg_device_ops ops;

/** Create/destroy namespace message */
Expand Down
2 changes: 1 addition & 1 deletion lib/remoteproc/remoteproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ void remoteproc_init_mem(struct remoteproc_mem *mem, const char *name,
if (!mem || !io || size == 0)
return;
if (name)
strncpy(mem->name, name, sizeof(mem->name));
strlcpy(mem->name, name, sizeof(mem->name));
else
mem->name[0] = 0;
mem->pa = pa;
Expand Down
12 changes: 10 additions & 2 deletions lib/rpmsg/rpmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ int rpmsg_send_ns_message(struct rpmsg_endpoint *ept, unsigned long flags)

ns_msg.flags = flags;
ns_msg.addr = ept->addr;
strncpy(ns_msg.name, ept->name, sizeof(ns_msg.name));
strlcpy(ns_msg.name, ept->name, sizeof(ns_msg.name));
Copy link
Collaborator

Choose a reason for hiding this comment

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

ept->name is safe as already truncated in rpmsg_register_endpoint, we could use a memcpy here.

ret = rpmsg_send_offchannel_raw(ept, ept->addr,
RPMSG_NS_EPT_ADDR,
&ns_msg, sizeof(ns_msg), true);
Expand Down Expand Up @@ -305,7 +305,7 @@ void rpmsg_register_endpoint(struct rpmsg_device *rdev,
rpmsg_ept_cb cb,
rpmsg_ns_unbind_cb ns_unbind_cb, void *priv)
{
strncpy(ept->name, name ? name : "", sizeof(ept->name));
strlcpy(ept->name, name ? name : "", sizeof(ept->name));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should create our own strncpy.
we could create a libutil/strncpy.c that contains a safe_strncpy function.

char *safe_strncpy(char *dest, char *src, size_t size) {
	size_t len;

	if (size == 0) {
		dest[0] = '\0';
	} else {	
		len = strnlen(src, size - 1);
		memcpy(dest, src, len);
		dest[len] = '\0';
	}
	return dest;
}

@edmooring, @tnmysh : any opinon/suggestion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe that makes sense. In kernel as well, strlcpy was deprecated, and strscpy was introduced. https://www.kernel.org/doc/html/latest/core-api/kernel-api.html?highlight=declare_bitmap#c.strscpy

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am okay, to introduce such function.

ept->refcnt = 1;
ept->addr = src;
ept->dest_addr = dest;
Expand Down Expand Up @@ -385,3 +385,11 @@ void rpmsg_destroy_ept(struct rpmsg_endpoint *ept)
(void)rpmsg_send_ns_message(ept, RPMSG_NS_DESTROY);
rpmsg_unregister_endpoint(ept);
}

int rpmsg_notify_wait(struct rpmsg_device *rdev, uint32_t id)
{
if (rdev->notify_wait_cb)
return rdev->notify_wait_cb(rdev, id);

return RPMSG_ERR_NXIO;
}
2 changes: 2 additions & 0 deletions lib/rpmsg/rpmsg_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ rpmsg_get_ept_from_addr(struct rpmsg_device *rdev, uint32_t addr)
return rpmsg_get_endpoint(rdev, NULL, addr, RPMSG_ADDR_ANY);
}

int rpmsg_notify_wait(struct rpmsg_device *rdev, uint32_t id);

/**
* @internal
*
Expand Down
8 changes: 3 additions & 5 deletions lib/rpmsg/rpmsg_virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,14 @@ static void rpmsg_virtio_release_rx_buffer(struct rpmsg_device *rdev,
metal_mutex_release(&rdev->lock);
}

static int rpmsg_virtio_notify_wait(struct rpmsg_virtio_device *rvdev, struct virtqueue *vq)
static int rpmsg_virtio_notify_wait(struct rpmsg_virtio_device *rvdev,
struct virtqueue *vq)
{
struct virtio_vring_info *vring_info;

vring_info = &rvdev->vdev->vrings_info[vq->vq_queue_index];

if (!rvdev->notify_wait_cb)
return RPMSG_EOPNOTSUPP;

return rvdev->notify_wait_cb(&rvdev->rdev, vring_info->notifyid);
return rpmsg_notify_wait(&rvdev->rdev, vring_info->notifyid);
}

static void *rpmsg_virtio_get_tx_payload_buffer(struct rpmsg_device *rdev,
Expand Down
Loading