07 Oct, 2009

1 commit

  • trivial bug in fs/cifs/connect.c .
    The bug is caused by fail of extract_hostname()
    when mounting cifs file system.

    This is the situation when I noticed this bug.

    % sudo mount -t cifs //192.168.10.208 mountpoint -o options...

    Then my kernel says,

    [ 1461.807776] ------------[ cut here ]------------
    [ 1461.807781] kernel BUG at mm/slab.c:521!
    [ 1461.807784] invalid opcode: 0000 [#2] PREEMPT SMP
    [ 1461.807790] last sysfs file:
    /sys/devices/pci0000:00/0000:00:1e.0/0000:09:02.0/resource
    [ 1461.807793] CPU 0
    [ 1461.807796] Modules linked in: nls_iso8859_1 usbhid sbp2 uhci_hcd
    ehci_hcd i2c_i801 ohci1394 ieee1394 psmouse serio_raw pcspkr sky2 usbcore
    evdev
    [ 1461.807816] Pid: 3446, comm: mount Tainted: G D 2.6.32-rc2-vanilla
    [ 1461.807820] RIP: 0010:[] []
    kfree+0x63/0x156
    [ 1461.807829] RSP: 0018:ffff8800b4f7fbb8 EFLAGS: 00010046
    [ 1461.807832] RAX: ffffea00033fff98 RBX: ffff8800afbae7e2 RCX:
    0000000000000000
    [ 1461.807836] RDX: ffffea0000000000 RSI: 000000000000005c RDI:
    ffffffffffffffea
    [ 1461.807839] RBP: ffff8800b4f7fbf8 R08: 0000000000000001 R09:
    0000000000000000
    [ 1461.807842] R10: 0000000000000000 R11: ffff8800b4f7fbf8 R12:
    00000000ffffffea
    [ 1461.807845] R13: ffff8800afb23000 R14: ffff8800b4f87bc0 R15:
    ffffffffffffffea
    [ 1461.807849] FS: 00007f52b6f187c0(0000) GS:ffff880007600000(0000)
    knlGS:0000000000000000
    [ 1461.807852] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
    [ 1461.807855] CR2: 0000000000613000 CR3: 00000000af8f9000 CR4:
    00000000000006f0
    [ 1461.807858] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
    0000000000000000
    [ 1461.807861] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7:
    0000000000000400
    [ 1461.807865] Process mount (pid: 3446, threadinfo ffff8800b4f7e000, task
    ffff8800950e4380)
    [ 1461.807867] Stack:
    [ 1461.807869] 0000000000000202 0000000000000282 ffff8800b4f7fbf8
    ffff8800afbae7e2
    [ 1461.807876] 00000000ffffffea ffff8800afb23000 ffff8800b4f87bc0
    ffff8800b4f7fc28
    [ 1461.807884] ffff8800b4f7fcd8 ffffffff81159f6d ffffffff81147bc2
    ffffffff816bfb48
    [ 1461.807892] Call Trace:
    [ 1461.807899] [] cifs_get_tcp_session+0x440/0x44b
    [ 1461.807904] [] ? find_nls+0x1c/0xe9
    [ 1461.807909] [] cifs_mount+0x16bc/0x2167
    [ 1461.807917] [] ? _spin_unlock+0x30/0x4b
    [ 1461.807923] [] cifs_get_sb+0xa5/0x1a8
    [ 1461.807928] [] vfs_kern_mount+0x56/0xc9
    [ 1461.807933] [] do_kern_mount+0x47/0xe7
    [ 1461.807938] [] do_mount+0x712/0x775
    [ 1461.807943] [] ? copy_mount_options+0xcf/0x132
    [ 1461.807948] [] sys_mount+0x7f/0xbf
    [ 1461.807953] [] ? lockdep_sys_exit_thunk+0x35/0x67
    [ 1461.807960] [] system_call_fastpath+0x16/0x1b
    [ 1461.807963] Code: 00 00 00 00 ea ff ff 48 c1 e8 0c 48 6b c0 68 48 01 d0
    66 83 38 00 79 04 48 8b 40 10 66 83 38 00 79 04 48 8b 40 10 80 38 00 78 04
    0b eb fe 4c 8b 70 58 4c 89 ff 41 8b 76 4c e8 b8 49 fb ff e8
    [ 1461.808022] RIP [] kfree+0x63/0x156
    [ 1461.808027] RSP
    [ 1461.808031] ---[ end trace ffe26fcdc72c0ce4 ]---

    The reason of this bug is that the error handling code of
    cifs_get_tcp_session()
    calls kfree() when corresponding kmalloc() failed.
    (The kmalloc() is called by extract_hostname().)

    Signed-off-by: Hitoshi Mitake
    CC: Stable
    Reviewed-by: Jeff Layton
    Signed-off-by: Steve French

    Steve French
     

27 Sep, 2009

1 commit

  • * git://git.kernel.org/pub/scm/linux/kernel/git/sfrench/cifs-2.6:
    cifs: fix locking and list handling code in cifs_open and its helper
    [CIFS] Remove build warning
    cifs: fix problems with last two commits
    [CIFS] Fix build break when keys support turned off
    cifs: eliminate cifs_init_private
    cifs: convert oplock breaks to use slow_work facility (try #4)
    cifs: have cifsFileInfo hold an extra inode reference
    cifs: take read lock on GlobalSMBSes_lock in is_valid_oplock_break
    cifs: remove cifsInodeInfo.oplockPending flag
    cifs: fix oplock request handling in posix codepath
    [CIFS] Re-enable Lanman security

    Linus Torvalds
     

26 Sep, 2009

1 commit

  • The patch to remove cifs_init_private introduced a locking imbalance. It
    didn't remove the leftover list addition code and the unlocking in that
    function. cifs_new_fileinfo does the list addition now, so there should
    be no need to do it outside of that function.

    pCifsInode will never be NULL, so we don't need to check for that. This
    patch also gets rid of the ugly locking and unlocking across function
    calls.

    Signed-off-by: Jeff Layton
    Acked-by: Steve French
    Signed-off-by: Steve French

    Jeff Layton
     

25 Sep, 2009

5 commits

  • Acked-by: Jeff Layton
    Signed-off-by: Steve French

    Steve French
     
  • Fix problems with commits:

    086f68bd97126618ecb2dcff5f766f3a21722df7
    3bc303c254335dbd7c7012cc1760b12f1d5514d3

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • Acked-by: Jeff Layton
    Signed-off-by: Steve French

    Steve French
     
  • ...it does the same thing as cifs_fill_fileinfo, but doesn't handle the
    flist ordering correctly. Also rename cifs_fill_fileinfo to a more
    descriptive name and have it take an open flags arg instead of just a
    write_only flag. That makes the logic in the callers a little simpler.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • This is the fourth respin of the patch to convert oplock breaks to
    use the slow_work facility.

    A customer of ours was testing a backport of one of the earlier
    patchsets, and hit a "Busy inodes after umount..." problem. An oplock
    break job had raced with a umount, and the superblock got torn down and
    its memory reused. When the oplock break job tried to dereference the
    inode->i_sb, the kernel oopsed.

    This patchset has the oplock break job hold an inode and vfsmount
    reference until the oplock break completes. With this, there should be
    no need to take a tcon reference (the vfsmount implicitly holds one
    already).

    Currently, when an oplock break comes in there's a chance that the
    oplock break job won't occur if the allocation of the oplock_q_entry
    fails. There are also some rather nasty races in the allocation and
    handling these structs.

    Rather than allocating oplock queue entries when an oplock break comes
    in, add a few extra fields to the cifsFileInfo struct. Get rid of the
    dedicated cifs_oplock_thread as well and queue the oplock break job to
    the slow_work thread pool.

    This approach also has the advantage that the oplock break jobs can
    potentially run in parallel rather than be serialized like they are
    today.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     

24 Sep, 2009

2 commits

  • Update some fs code to make use of new helper functions introduced
    in the previous patch. Should be no significant change in behaviour
    (except CIFS now calls send_sig under i_lock, via inode_newsize_ok).

    Reviewed-by: Christoph Hellwig
    Acked-by: Miklos Szeredi
    Cc: linux-nfs@vger.kernel.org
    Cc: Trond.Myklebust@netapp.com
    Cc: linux-cifs-client@lists.samba.org
    Cc: sfrench@samba.org
    Signed-off-by: Nick Piggin
    Signed-off-by: Al Viro

    npiggin@suse.de
     
  • Most call sites of unload_nls() do:
    if (nls)
    unload_nls(nls);

    Check the pointer inside unload_nls() like we do in kfree() and
    simplify the call sites.

    Signed-off-by: Thomas Gleixner
    Cc: Steve French
    Cc: OGAWA Hirofumi
    Cc: Roman Zippel
    Cc: Dave Kleikamp
    Cc: Petr Vandrovec
    Cc: Anton Altaparmakov
    Signed-off-by: Al Viro

    Thomas Gleixner
     

22 Sep, 2009

3 commits

  • * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial: (34 commits)
    trivial: fix typo in aic7xxx comment
    trivial: fix comment typo in drivers/ata/pata_hpt37x.c
    trivial: typo in kernel-parameters.txt
    trivial: fix typo in tracing documentation
    trivial: add __init/__exit macros in drivers/gpio/bt8xxgpio.c
    trivial: add __init macro/ fix of __exit macro location in ipmi_poweroff.c
    trivial: remove unnecessary semicolons
    trivial: Fix duplicated word "options" in comment
    trivial: kbuild: remove extraneous blank line after declaration of usage()
    trivial: improve help text for mm debug config options
    trivial: doc: hpfall: accept disk device to unload as argument
    trivial: doc: hpfall: reduce risk that hpfall can do harm
    trivial: SubmittingPatches: Fix reference to renumbered step
    trivial: fix typos "man[ae]g?ment" -> "management"
    trivial: media/video/cx88: add __init/__exit macros to cx88 drivers
    trivial: fix typo in CONFIG_DEBUG_FS in gcov doc
    trivial: fix missing printk space in amd_k7_smp_check
    trivial: fix typo s/ketymap/keymap/ in comment
    trivial: fix typo "to to" in multiple files
    trivial: fix typos in comments s/DGBU/DBGU/
    ...

    Linus Torvalds
     
  • Signed-off-by: Alexey Dobriyan
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Alexey Dobriyan
     
  • Signed-off-by: Alexey Dobriyan
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Alexey Dobriyan
     

21 Sep, 2009

1 commit


16 Sep, 2009

5 commits

  • It's possible that this struct will outlive the filp to which it is
    attached. If it does and it needs to do some work on the inode, then
    it'll need a reference.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • ...rather than a write lock. It doesn't change the list so a read lock
    should be sufficient.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • It's set on oplock break but nothing ever looks at it.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • cifs_posix_open takes a "poplock" argument that's intended to be used in
    the actual posix open call to set the "Flags" field. It ignores this
    value however and declares an "oplock" parameter on the stack that it
    passes uninitialized to the CIFSPOSIXOpen function. Not only does this
    mean that the oplock request flags are bogus, but the result that's
    expected to be in that variable is unchanged.

    Fix this, and also clean up the type of the oplock parameter used. Since
    it's expected to be __u32, we should use that everywhere and not
    implicitly cast it from a signed type.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • commit ac68392460ffefed13020967bae04edc4d3add06 ("[CIFS] Allow raw
    ntlmssp code to be enabled with sec=ntlmssp") added a new bit to the
    allowed security flags mask but seems to have inadvertently removed
    Lanman security from the allowed flags. Add it back.

    CC: Stable
    Signed-off-by: Chuck Ebbert
    Signed-off-by: Steve French

    Chuck Ebbert
     

03 Sep, 2009

1 commit


02 Sep, 2009

7 commits

  • Currently, cifs_close() tries to wait until all I/O is complete and then
    frees the file private data. If I/O does not completely in a reasonable
    amount of time it frees the structure anyway, leaving a potential use-
    after-free situation.

    This patch changes the wrtPending counter to a complete reference count and
    lets the last user free the structure.

    Signed-off-by: Dave Kleikamp
    Reviewed-by: Jeff Layton
    Tested-by: Shirish Pargaonkar
    Signed-off-by: Steve French

    Dave Kleikamp
     
  • Right now, the GlobalOplock_Q is protected by the GlobalMid_Lock. That
    lock is also used for completely unrelated purposes (mostly for managing
    the global mid queue). Give the list its own dedicated spinlock
    (cifs_oplock_lock) and rename the list to cifs_oplock_list to
    eliminate the camel-case.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • Minor nit: we already have a tcon pointer so we don't need to
    dereference cifs_sb again.

    Also initialize the vars in the declaration.

    Reported-by: Peter Staubach
    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • Make it easier on the upcall program by adding ':' delimiters between
    each group of hex digits.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • Also update version number to 1.61

    Signed-off-by: Steve French

    Steve French
     
  • One more try..

    It seems there is a regression that got introduced while Jeff fixed
    all the mount/umount races. While attempting to find whether a tcp
    session is already existing, we were not checking whether the "port"
    used are the same. When a second mount is attempted with a different
    "port=" option, it is being ignored. Because of this the cifs mounts
    that uses a SSH tunnel appears to be broken.

    Steps to reproduce:

    1. create 2 shares
    # SSH Tunnel a SMB session
    2. ssh -f -L 6111:127.0.0.1:445 root@localhost "sleep 86400"
    3. ssh -f -L 6222:127.0.0.1:445 root@localhost "sleep 86400"
    4. tcpdump -i lo 6111 &
    5. mkdir -p /mnt/mnt1
    6. mkdir -p /mnt/mnt2
    7. mount.cifs //localhost/a /mnt/mnt1 -o username=guest,ip=127.0.0.1,port=6111
    #(shows tcpdump activity on port 6111)
    8. mount.cifs //localhost/b /mnt/mnt2 -o username=guest,ip=127.0.0.1,port=6222
    #(shows tcpdump activity only on port 6111 and not on 6222

    Fix by adding a check to compare the port _only_ if the user tries to
    override the tcp port with "port=" option, before deciding that an
    existing tcp session is found. Also, clean up a bit by replacing
    if-else if by a switch statment while at it as suggested by Jeff.

    Reviewed-by: Jeff Layton
    Reviewed-by: Shirish Pargaonkar
    Signed-off-by: Suresh Jayaraman
    Signed-off-by: Steve French

    Suresh Jayaraman
     
  • in function calc_ntlmv2_hash memory is not released.
    1. If in the line 333 we successfully allocate memory and assign it to
    pctxt variable:
    pctxt = kmalloc(sizeof(struct HMACMD5Context), GFP_KERNEL);
    then we go to line 376 and exit wihout releasing memory pointed to by pctxt
    variable.

    Add a memory releasing for pctxt variable before exit from function
    calc_ntlmv2_hash.

    Signed-off-by: Alexander Strakh
    Signed-off-by: Steve French

    Alexander Strakh
     

31 Aug, 2009

1 commit


04 Aug, 2009

3 commits


02 Aug, 2009

1 commit

  • This patch fixes the regression reported here:

    http://bugzilla.kernel.org/show_bug.cgi?id=13861

    commit 4ae1507f6d266d0cc3dd36e474d83aad70fec9e4 changed the default
    behavior when the uid= or gid= option was specified for a mount. The
    existing behavior was to always clobber the ownership information
    provided by the server when these options were specified. The above
    commit changed this behavior so that these options simply provided
    defaults when the server did not provide this information (unless
    "forceuid" or "forcegid" were specified)

    This patch reverts this change so that the default behavior is restored.
    It also adds "noforceuid" and "noforcegid" options to make it so that
    ownership information from the server is preserved, even when the mount
    has uid= or gid= options specified.

    It also adds a couple of printk notices that pop up when forceuid or
    forcegid options are specified without a uid= or gid= option.

    Reported-by: Tom Chiverton
    Reviewed-by: Shirish Pargaonkar
    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     

30 Jul, 2009

1 commit


28 Jul, 2009

1 commit


23 Jul, 2009

3 commits

  • Signed-off-by: Steve French

    Steve French
     
  • This off-by-one bug causes sendfile() to not work properly. When a task
    calls sendfile() on a file on a CIFS filesystem, the syscall returns -1
    and sets errno to EOVERFLOW.

    do_sendfile uses s_maxbytes to verify the returned offset of the file.
    The problem there is that this value is cast to a signed value (loff_t).
    When this is done on the s_maxbytes value that cifs uses, it becomes
    negative and the comparisons against it fail.

    Even though s_maxbytes is an unsigned value, it seems that it's not OK
    to set it in such a way that it'll end up negative when it's cast to a
    signed value. These casts happen in other codepaths besides sendfile
    too, but the VFS is a little hard to follow in this area and I can't
    be sure if there are other bugs that this will fix.

    It's not clear to me why s_maxbytes isn't just declared as loff_t in the
    first place, but either way we still need to fix these values to make
    sendfile work properly. This is also an opportunity to replace the magic
    bit-shift values here with the standard #defines for this.

    This fixes the reproducer program I have that does a sendfile and
    will probably also fix the situation where apache is serving from a
    CIFS share.

    Acked-by: Johannes Weiner
    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     
  • A recent regression when dealing with older servers. This bug was
    introduced when we made serverino the default...

    When the server can't provide inode numbers, disable it for the mount.

    Signed-off-by: Jeff Layton
    Signed-off-by: Steve French

    Jeff Layton
     

21 Jul, 2009

1 commit


10 Jul, 2009

2 commits