Hi Christian, On 14/09/2026 12:06, Christian König wrote: > On 9/11/26 23:41, Matt Evans wrote: >> dma_buf_set_name() originally took a __user string to duplicate for >> the buffer name. Make this function a generic set-name helper, taking >> a kernel-allocated string. >> >> Wrap this by a new dma_buf_set_name_user() to support the existing >> ioctl path for a user-provided string. > > That just describes what the patch does and not why, in other words this > needs a bit more justification for the change. > > For example something like "Allow exporters/importers to set the name of > buffers from pre-existing information".
Fair enough, am adding missing rationale. > It is also important to describe which role should set a name (exporter, > importer or both). When an exporter gives a standard name for it's buffers > that is most likely harmless, but when an importer bluntly overwrites a name > previously set by an exporter or userspace then we really need a good reason > for that. Good point; my expectation was that exporters would use this to either set or update the name, and importers would never do this. I will make this clear in the commit message/comments, thanks. >> >> Signed-off-by: Matt Evans <[email protected]> >> --- >> drivers/dma-buf/dma-buf.c | 58 ++++++++++++++++++++++++++++++--------- >> include/linux/dma-buf.h | 2 ++ >> 2 files changed, 47 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >> index d504c636dc29..8129ea11ff58 100644 >> --- a/drivers/dma-buf/dma-buf.c >> +++ b/drivers/dma-buf/dma-buf.c >> @@ -405,31 +405,29 @@ static __poll_t dma_buf_poll(struct file *file, >> poll_table *poll) >> } >> >> /** >> - * dma_buf_set_name - Set a name to a specific dma_buf to track the usage. >> - * It could support changing the name of the dma-buf if the same >> - * piece of memory is used for multiple purpose between different devices. >> + * dma_buf_set_name_user - Set a dma_buf's name from a user string >> + * >> + * The string is up to DMA_BUF_NAME_LEN long, including the terminator. >> * >> * @dmabuf: [in] dmabuf buffer that will be renamed. >> * @buf: [in] A piece of userspace memory that contains the name of >> * the dma-buf. >> * >> - * Returns 0 on success. If the dma-buf buffer is already attached to >> - * devices, return -EBUSY. >> - * >> + * Returns 0 on success, and any previously-set name is freed. >> */ > > Kerneldoc for a static function is usually overkill. The original static function had some so I kept it, but no worries. Removed. > And since this is used only once and very complicated I would completely > merge the logic into dma_buf_ioctl(). Will do (no function, no kerneldoc). Many thanks, Matt > > Regards, > Christian. > >> -static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) >> +static long dma_buf_set_name_user(struct dma_buf *dmabuf, const char __user >> *buf) >> { >> char *name = strndup_user(buf, DMA_BUF_NAME_LEN); >> + int ret; >> >> if (IS_ERR(name)) >> return PTR_ERR(name); >> >> - spin_lock(&dmabuf->name_lock); >> - kfree(dmabuf->name); >> - dmabuf->name = name; >> - spin_unlock(&dmabuf->name_lock); >> + ret = dma_buf_set_name(dmabuf, name); >> + if (ret) >> + kfree(name); >> >> - return 0; >> + return ret; >> } >> >> #if IS_ENABLED(CONFIG_SYNC_FILE) >> @@ -578,7 +576,7 @@ static long dma_buf_ioctl(struct file *file, >> >> case DMA_BUF_SET_NAME_A: >> case DMA_BUF_SET_NAME_B: >> - return dma_buf_set_name(dmabuf, (const char __user *)arg); >> + return dma_buf_set_name_user(dmabuf, (const char __user *)arg); >> >> #if IS_ENABLED(CONFIG_SYNC_FILE) >> case DMA_BUF_IOCTL_EXPORT_SYNC_FILE: >> @@ -854,6 +852,40 @@ void dma_buf_put(struct dma_buf *dmabuf) >> } >> EXPORT_SYMBOL_NS_GPL(dma_buf_put, "DMA_BUF"); >> >> +/** >> + * dma_buf_set_name - Set a dma_buf's name >> + * It could support changing the name of the dma-buf if the same piece >> + * of memory is used for multiple purpose between different devices. >> + * >> + * @dmabuf: [in] dmabuf buffer that will be renamed. >> + * @name: [in] The name of the dma-buf, allocated with kmalloc() or >> + * similar. This takes ownership of the allocation >> + * on success, which will be kfree()d when the >> + * dmabuf is released or a new name assigned. >> + * >> + * Returns 0 on success, -EINVAL if the name is NULL, or -E2BIG if the >> + * name exceeds DMA_BUF_NAME_LEN. >> + */ >> +int dma_buf_set_name(struct dma_buf *dmabuf, char *name) >> +{ >> + if (!name) >> + return -EINVAL; >> + >> + /* dmabuffs_dname() won't use the string if the length >> + * (including terminator) exceeds DMA_BUF_NAME_LEN: >> + */ >> + if (strlen(name) >= DMA_BUF_NAME_LEN) >> + return -E2BIG; >> + >> + spin_lock(&dmabuf->name_lock); >> + kfree(dmabuf->name); >> + dmabuf->name = name; >> + spin_unlock(&dmabuf->name_lock); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_NS_GPL(dma_buf_set_name, "DMA_BUF"); >> + >> static int dma_buf_wrap_sg_table(struct sg_table **sg_table) >> { >> struct scatterlist *to_sg, *from_sg; >> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h >> index d1203da56fc5..14d3950b63c8 100644 >> --- a/include/linux/dma-buf.h >> +++ b/include/linux/dma-buf.h >> @@ -570,6 +570,8 @@ int dma_buf_fd(struct dma_buf *dmabuf, int flags); >> struct dma_buf *dma_buf_get(int fd); >> void dma_buf_put(struct dma_buf *dmabuf); >> >> +int dma_buf_set_name(struct dma_buf *dmabuf, char *name); >> + >> struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *, >> enum dma_data_direction); >> void dma_buf_unmap_attachment(struct dma_buf_attachment *, struct sg_table >> *, >
