From owner-freebsd-fs@freebsd.org Tue May 30 19:27:23 2017 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A2E0DBDA1A4 for ; Tue, 30 May 2017 19:27:23 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-qk0-x22a.google.com (mail-qk0-x22a.google.com [IPv6:2607:f8b0:400d:c09::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 6AF6B760E8 for ; Tue, 30 May 2017 19:27:23 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-qk0-x22a.google.com with SMTP id u75so76836031qka.3 for ; Tue, 30 May 2017 12:27:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=x//YYnRc5vdJuwcrSORs41aNoZrcjW7meLGG37s9304=; b=CXsEfLereuw/eKdkhKGP68GsAWSBwzlGBPzU2LcjW2Og8D1qy//idbb9bo3NPXPtch 2rnhAQTMMIMX0AIqiNZ9QZtfSVxK2s/GLpz/qH3bfTec/h9yc5euXb/Bl7ZSNRh1QkYr hBL242P6DoJ0EkKYkY8oa1PAe4CV5hXc8xzAPYrODXJV4kKQLfGrEvMrxGVMhkjNeX/O gd7EVbzR5YHBWiLwPXhIQQYroSA4RwuovhPBPKknHMpIwEzooX2HECpHd4fDqNiN0BaB zAFOEo9GDBrtEGal7tgjJwePGMApOD7oxFOhJUxtK4kK8jd2PhZGtfwbNotAl6sJO5BP yoJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=x//YYnRc5vdJuwcrSORs41aNoZrcjW7meLGG37s9304=; b=Jmu7cA5rx3fDrAwGbKMG0u6p2N9XIGbGMFlBBIlHr1le0+U1wBblCOOFCR97QqS3zV 5+d84ZQ1e3hOoUKEYdiUuLosjeB6ITnyWJOQ4MraGkskbcYtkUUEHpIfjvUfP3J6BH1u acPTZRiIYxxbq5NfK++8bcDOf1gKyz6Mqvet7hGR042evvTYeZg1b3J7j6GTcvZYaqBF Irgq7GUcN7G9Dj1eSbTH7IrO2l1Px8NPU9XEecOFLE4i8sNh4uzSVTPz6tfyckKo4hcE Je2oLbmN7t3PmqG8WBdShBEgl5xk55r6JFeXKsByUFyNEn3hdI705d3KlTJ0jxGm7csm stAA== X-Gm-Message-State: AODbwcAN7bIOqcCm7sS2ZZm9ZGRx4WEqoTFdkgwg0tMQtX9I7kwFCHoO LCPW8cd7oQcJ08hpO6kus9ZNluRrag== X-Received: by 10.55.217.70 with SMTP id u67mr24621311qki.17.1496172442521; Tue, 30 May 2017 12:27:22 -0700 (PDT) MIME-Version: 1.0 Received: by 10.237.61.154 with HTTP; Tue, 30 May 2017 12:27:22 -0700 (PDT) In-Reply-To: <38666c423c33a5e1009c106c23aeb218@nixd.org> References: <38666c423c33a5e1009c106c23aeb218@nixd.org> From: Mateusz Guzik Date: Tue, 30 May 2017 21:27:22 +0200 Message-ID: Subject: Re: VFS vn_lock function makes system unresponsive when calling vn_fullpath To: Alexander Morozov Cc: "freebsd-fs@freebsd.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 May 2017 19:27:23 -0000 On Tue, May 30, 2017 at 4:42 PM, Alexander Morozov wrote: > Hi, > > At the moment I am developing a kernel module based on MAC Framework which > is invoking a vn_fullpath() from vfs_cache.c. > > Here is the thing: > When the MAC Framework mpo_vnode_check_write procedure is called, the > kernel module is trying to retrieve: the path on the disk for the curthread > and the path of the file in which the curthread is attempting to write. At > the point when the program execution reaches the vn_fullpath() for the > resolution of the file's path, the terminal window 'freezes' (actually the > whole system is not responding, the rest ttys stop responding after > entering login credentials). > > For instance, after loading and initializing a MAC kernel module, I am > opening a existing text file using vi to edit it. After inserting some > random text I press ESC key on keyboard and terminal window 'freezes' (at > that moment the mpo_vnode_check_write is called) ( the struct vnode * vp > which is passed to the MAC procedure is valid (not NULL) and type (enum > vtype) is VREG). > > I have investigated this issue and found out the following: > The get_fullpath() is calling the get_fullpath1() where later the > vn_vptocnp() is invoked. > Retrieving the location for the curthread is successful (the full path > returned). > But when the kernel module is making attempt to retrieve the path for the > vp (argument of the mpo_vnode_check_write) the function vn_lock(*vp, > LK_SHARED | LK_RETRY) (line 2191) located in function vn_vptocnp() is > grabbing control forever. > Hard to say off hand, but so far it looks like the vnode is already exclusively locked and now the kernel deadlocks itself by locking it in shared mode. You can easily inspect the state in ddb with show lockedvnods. Are you running the kernel with DEBUG_VFS_LOCKS? What is the purpose of this module in the first place? regular vnode -> path resolution is not guaranteed to work. While it will work most of the time, it is inherently racy problematic in presence of multiple hardlinks. For instance someone else can be modifying the directory tree as you translate back and trick you into thinking the vnode represents a different file. Even if this was not the case, the translation of the sort on each write would be a performance killer. The only possibly working approach I see would attach metadata to the vnode after lookup and then use it. > Below I have copied and pasted the code which performers the path > resolution and the MAC procedure handler: > static int > rw_retreive_data(struct thread * td, struct vnode *dvp, char ** rpath, > char ** curpath, struct sandbox_rule_app ** rule_ptr) > { > [snip] > error = vn_fullpath(td, td->td_proc->p_textvp, curpath, &curfreepath); > [snip] > } > > int sandbox_vnode_check_write(struct ucred *active_cred, > struct > ucred *file_cred, > struct > vnode *vp, > struct > label *vplabel) > { > IS_MODULE_INITED(0) > ASSERT_NULL_R(vp, 0); > > struct sandbox_rule_app * rule_ptr = NULL; > char * rpath = "-"; > char * curpath = "-"; > int error = 0; > > RWLOCK_BLOCK(&sandbox_rules_lock, RWLOCK_READ) > { > > error = rw_retreive_data(curthread, vp, &rpath, &curpath, > &rule_ptr); > If this is using rwlock the code is additionally wrong as vn_fullpath can induce unbound sleep, while the lock at hand only supports bound sleep. -- Mateusz Guzik