diff mbox series

fs: ext4: Fixed file permissions

Message ID 20240314064130.25113-1-Jixiong.Hu@mediatek.com
State Changes Requested
Delegated to: Tom Rini
Headers show
Series fs: ext4: Fixed file permissions | expand

Commit Message

Jixiong Hu March 14, 2024, 6:41 a.m. UTC 
Modified the ext4fs_write function to create a new file that
inherits the inode->mode of existing file. To fix an issue
where file permissions are changed after modifying the contents
of an existing file.
---
 fs/ext4/ext4_write.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

2.18.0

Comments

Dan Carpenter March 14, 2024, 11:29 a.m. UTC | #1 
On Thu, Mar 14, 2024 at 02:41:29PM +0800, Jixiong Hu wrote:
> Modified the ext4fs_write function to create a new file that
> inherits the inode->mode of existing file. To fix an issue
> where file permissions are changed after modifying the contents
> of an existing file.

I'm not an expert, but honestly the current behavior sound more correct
than the proposed behavior.  What's the issue specifically so we can
understand it better?  Also bug fixes need a Fixes tag.

The patch has a number of style issues and the existing_file_inode
allocation is not always freed so those memory leaks would need to be
fixed.  For the style issues run ./scripts/checkpatch.pl on your patch.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index ea4c5d4157..6da4a26b71 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -847,6 +847,7 @@  int ext4fs_write(const char *fname, const char *buffer,
 {
 	int ret = 0;
 	struct ext2_inode *file_inode = NULL;
+	struct ext2_inode *existing_file_inode = NULL;
 	unsigned char *inode_buffer = NULL;
 	int parent_inodeno;
 	int inodeno;
@@ -900,6 +901,16 @@  int ext4fs_write(const char *fname, const char *buffer,
 	/* check if the filename is already present in root */
 	existing_file_inodeno = ext4fs_filename_unlink(filename);
 	if (existing_file_inodeno != -1) {
+		existing_file_inode = (struct ext2_inode *)zalloc(fs->inodesz);
+		if (!existing_file_inode)
+			goto fail;
+		ret = ext4fs_iget(existing_file_inodeno, existing_file_inode);
+		if (ret)
+		{
+			free(existing_file_inode);
+			goto fail;
+		}
+
 		ret = ext4fs_delete_file(existing_file_inodeno);
 		fs->first_pass_bbmap = 0;
 		fs->curr_blkno = 0;
@@ -948,8 +959,13 @@  int ext4fs_write(const char *fname, const char *buffer,
 			sizebytes = 0;
 		}
 	} else {
-		file_inode->mode = cpu_to_le16(S_IFREG | S_IRWXU | S_IRGRP |
-					       S_IROTH | S_IXGRP | S_IXOTH);
+		if(existing_file_inode) {
+			file_inode->mode = existing_file_inode->mode;
+			free(existing_file_inode);
+		} else {
+			file_inode->mode = cpu_to_le16(S_IFREG | S_IRWXU | S_IRGRP |
+						       S_IROTH | S_IXGRP | S_IXOTH);
+		}
 	}
 	/* ToDo: Update correct time */
 	file_inode->mtime = cpu_to_le32(timestamp);