lkml.org 
[lkml]   [2004]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch 1/3] copy_to_user check in fs/cifs/file.c
On Sun, 26 Dec 2004, Alan Cox wrote:

> On Sul, 2004-12-26 at 23:24, Jesper Juhl wrote:
> > Hi,
> >
> > Patch below adds a check for the copy_to_user return value and makes a few
> > whitespace cleanups in fs/cifs/file.c::cifs_user_read()
> > I hope bundling two different things together in one patch is OK when the
> > change is as small as this, but if you want it spplit in two patches, then
> > just say so.
>
> Corrupts the stats
> Fails to free smb_read_data where in some cases it was freed before
>
> I'm not sure the stats matter but I think you need something more like
>
Like this patch perhaps? :

It implements what you sugested except it breaks the loop when there's
some residue from the copy_to_user call, since the logic after the while
loop handles the return total_read ? total_read : -EFAULT; case if we
set rc and break (I've cut the diff with a few lines of extra context to
make this easy to verify).

Comments welcome as always.


Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff --unified=8 -p linux-2.6.10-orig/fs/cifs/file.c linux-2.6.10/fs/cifs/file.c
--- linux-2.6.10-orig/fs/cifs/file.c 2004-12-24 22:33:48.000000000 +0100
+++ linux-2.6.10/fs/cifs/file.c 2004-12-28 23:27:17.000000000 +0100
@@ -1142,16 +1142,17 @@ cifs_user_read(struct file * file, char
open_file = (struct cifsFileInfo *)file->private_data;

if((file->f_flags & O_ACCMODE) == O_WRONLY) {
cFYI(1,("attempting read on write only file instance"));
}

for (total_read = 0,current_offset=read_data; read_size > total_read;
total_read += bytes_read,current_offset+=bytes_read) {
+ unsigned residue;
current_read_size = min_t(const int,read_size - total_read,cifs_sb->rsize);
rc = -EAGAIN;
smb_read_data = NULL;
while(rc == -EAGAIN) {
if ((open_file->invalidHandle) && (!open_file->closePend)) {
rc = cifs_reopen_file(file->f_dentry->d_inode,
file,TRUE);
if(rc != 0)
@@ -1159,22 +1160,27 @@ cifs_user_read(struct file * file, char
}

rc = CIFSSMBRead(xid, pTcon,
open_file->netfid,
current_read_size, *poffset,
&bytes_read, &smb_read_data);

pSMBr = (struct smb_com_read_rsp *)smb_read_data;
- copy_to_user(current_offset,smb_read_data + 4/* RFC1001 hdr*/
+ residue = copy_to_user(current_offset, smb_read_data + 4 /* RFC1001 hdr */
+ le16_to_cpu(pSMBr->DataOffset), bytes_read);
if(smb_read_data) {
cifs_buf_release(smb_read_data);
smb_read_data = NULL;
}
+ if (residue) {
+ total_read += bytes_read - residue;
+ rc = -EFAULT;
+ break;
+ }
}
if (rc || (bytes_read == 0)) {
if (total_read) {
break;
} else {
FreeXid(xid);
return rc;
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.099 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site