Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1756407Ab1FJMDa (ORCPT <rfc822;w@1wt.eu>);
	Fri, 10 Jun 2011 08:03:30 -0400
Received: from mail-iw0-f174.google.com ([209.85.214.174]:56083 "EHLO
	mail-iw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1755954Ab1FJMD2 convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 10 Jun 2011 08:03:28 -0400
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :cc:content-type:content-transfer-encoding;
        b=Ll4PUJDgZM1hsQlde1xZkxf3IvS72DmEdu+khiiLAquTGarvJUBhbULDYa3v5AacnU
         fDRh/r9mgcRvsP7z7UpQMXJkUtiq3b+t+zWGHcy/q9Wdguv92CbFgeXl2GCZnpx8R2Q8
         fcXoFb3b4qZP76nrOwEyz2NxW4dbG3zvxdvpI=
MIME-Version: 1.0
In-Reply-To: <20110610073702.061bc014@corrin.poochiereds.net>
References: <Pine.LNX.4.64.1106092211500.19687@sarijopen.student.utwente.nl>
	<20110609183045.01f6f9fc@tlielax.poochiereds.net>
	<BANLkTimPqmkNJMaJE2_C5zD8ESFOEM3=hg@mail.gmail.com>
	<Pine.LNX.4.64.1106100243370.2168@sarijopen.student.utwente.nl>
	<20110610073702.061bc014@corrin.poochiereds.net>
Date: Fri, 10 Jun 2011 05:03:27 -0700
Message-ID: <BANLkTinqT0AXx9STWEzrw-yASTid5rb1Fg@mail.gmail.com>
Subject: Re: [OOPS] 3.0-rc1 cifs
From: Connor Hansen <cmdkhh@gmail.com>
To: Jeff Layton <jlayton@redhat.com>
Cc: Martijn Uffing <mp3project@sarijopen.student.utwente.nl>,
        linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org,
        sean finney <seanius@seanius.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 4095
Lines: 112

On Fri, Jun 10, 2011 at 4:37 AM, Jeff Layton <jlayton@redhat.com> wrote:
> On Fri, 10 Jun 2011 02:57:21 +0200 (CEST)
> ?Uffing <mp3project@sarijopen.student.utwente.nl> wrote:
>
>>
>> <snip>
>> > call in get_dfs_path()
>> > rc = CIFSTCon(xid, pSesInfo, temp_unc, NULL, nls_codepage);
>> >
>> > function header for CIFSTCon
>> > int ?CIFSTCon(unsigned int xid, struct cifs_ses *ses,
>> > ? ? ? ? ?const char *tree, struct cifs_tcon *tcon,
>> > ? ? ? ? ?const struct nls_table *nls_codepage)
>> >
>> > get_dfs_path() is passing struct cifs_tcon *tcon as NULL
>> >
>> > from config: ?CONFIG_CIFS_WEAK_PW_HASH=y
>> >
>> > in CIFSTCon
>> >
>> > #ifdef CONFIG_CIFS_WEAK_PW_HASH
>> > 3222 ? ? ? ? ? ? ? ? if ((global_secflags & CIFSSEC_MAY_LANMAN) &&
>> > 3223 ? ? ? ? ? ? ? ? ? ? (ses->server->secType == LANMAN))
>> > 3224 ? ? ? ? ? ? ? ? ? ? ? ? calc_lanman_hash(tcon->password,
>> > ses->server->cryptkey,
>> >
>> > in calc_lanman_hash tcon is dereferenced(tcon->password) without being
>> > checked if null
>> >
>> > 3225 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ses->server->sec_mode &
>> > 3226 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? SECMODE_PW_ENCRYPT ?
>> > true : false,
>> > 3227 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bcc_ptr);
>> > 3228 ? ? ? ? ? ? ? ? else
>> > 3229 #endif /* CIFS_WEAK_PW_HASH */
>> >
>> > Connor
>>
>>
>> Ave all
>>
>> I recompiled ?kernel 3.0-rc1 (hadn't enabled CONFIG_DEBUG_INFO=y) and put
>> the oops (with the new adresses) through gdb per instruction of Jeff. And
>> Connor was spot on!
>>
>> <qoute oops>
>> BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0
>> IP: [<ffffffffa041e286>] CIFSTCon+0xf6/0x4d0 [cifs]
>> </qoute oops>
>>
>> <qoute gdb>
>> This GDB was configured as "x86_64-linux-gnu".
>> For bug reporting instructions, please see:
>> <http://www.gnu.org/software/gdb/bugs/>...
>> Reading symbols from
>> /lib/modules/3.0.0-rc1-debug/kernel/fs/cifs/cifs.ko...done.
>> (gdb) list *(CIFSTCon+0xf6)
>> 0xc2b6 is in CIFSTCon (fs/cifs/connect.c:3230).
>> 3225 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ses->server->sec_mode &
>> 3226 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?SECMODE_PW_ENCRYPT ?
>> true : false,
>> 3227 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bcc_ptr);
>> 3228 ? ? ? ? ? ? ? ? ? ?else
>> 3229 ? ?#endif /* CIFS_WEAK_PW_HASH */
>> 3230 ? ? ? ? ? ? ? ? ? ?rc = SMBNTencrypt(tcon->password,
>> ses->server->cryptkey,
>> 3231 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bcc_ptr);
>> 3232
>> 3233 ? ? ? ? ? ? ? ? ? ?bcc_ptr += CIFS_AUTH_RESP_SIZE;
>> 3234 ? ? ? ? ? ? ? ? ? ?if (ses->capabilities & CAP_UNICODE) {
>> (gdb)
>> </qoute gdb>
>>
>
> (cc'ing Sean F. since I suspect this regression is due to his changes)
>
> Thanks for the analysis, Martijn and Connor...
>
> What sort of server are you mounting here? It looks like it's using
> share-level security, so it's either very old or is a samba server
> configured that way.
>
> I suspect that commit c1508ca236 is the culprit. With that, we call
> into expand_dfs_referral on every mount attempt. Previously we only
> called into there when we got back ?an EREMOTE error and that would
> have been unlikely on a share-level security connection.
>
> I think there are several possible solutions, but since Sean was in
> here most recently I'd like to have his opinion.

I don't know enough about cifs but this call in fs/cifs/connect.c

2268: rc = CIFSTCon(xid, pSesInfo, temp_unc, NULL, nls_codepage);

will always result in a null pointer derefence as CIFSTCon uses the
cifs_tcon struct for passwords without verification

either we need to check for tcon for null in CIFSTCon before use, or
change get_dfs_path to get the cifs_tcon info and pass it to the
CIFSTCon function. But again I don't know enough about cifs to know
the best approach

Connor
>
> --
> Jeff Layton <jlayton@redhat.com>
>
--
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/