From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C028EC43387 for ; Fri, 11 Jan 2019 21:22:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8FD4A21783 for ; Fri, 11 Jan 2019 21:22:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="STQDMuEE" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725776AbfAKVWl (ORCPT ); Fri, 11 Jan 2019 16:22:41 -0500 Received: from bombadil.infradead.org ([198.137.202.133]:42320 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725308AbfAKVWl (ORCPT ); Fri, 11 Jan 2019 16:22:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=lbPaxjy7T9210D+wIbu+Yk5tPXdP0oWTdI/GrHyK8XY=; b=STQDMuEEacfCXZB/ur/bskOmr 14aZTqhlPOOn8jg2VDXRNETAAhweqhJyp1MVBqYFc8JCUvB7vcX03V8Vmdeh8Mh3v+mDwNTwizPqQ YmJXrJ4FCGdTWSrgj01RUO3TfnR63+zw4I1dXdIIwKlKLnRV+22xdbXanrRkYfK0Myx7+ag6/hxYc sKLudIduA3gKyw9pMH6xGze0aYRRTGzFjVvzjVy3hodP4MDDjuIvnHZ4fP2O05osqqPAr62PoW41p q1Mmq74eT3o7q/1SUWwoILNcVcbjcKikeEN0W17VI1xqQ8tplP40kwBDzMWSwq1/MbHKvqPP7sRFB Kr916KmBw==; Received: from willy by bombadil.infradead.org with local (Exim 4.90_1 #2 (Red Hat Linux)) id 1gi4GH-0002B5-3P; Fri, 11 Jan 2019 21:22:41 +0000 Date: Fri, 11 Jan 2019 13:22:40 -0800 From: Matthew Wilcox To: Steve French Cc: CIFS , linux-fsdevel , Pavel Shilovsky Subject: Re: scp bug due to progress indicator when copying from remote to local on Linux Message-ID: <20190111212240.GL6310@bombadil.infradead.org> References: <20190111132817.GE6310@bombadil.infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Fri, Jan 11, 2019 at 03:13:05PM -0600, Steve French wrote: > On Fri, Jan 11, 2019 at 7:28 AM Matthew Wilcox wrote: > > Are you saying the SIGALRM interrupts ftruncate() and causes the ftruncate > > to fail? > > So ftruncate does not really fail (the file contents and size match on > source and target after the copy) but the scp 'fails' and the user > would be quite confused (and presumably the network stack doesn't like > this signal, which can cause disconnects etc. which in theory could > cause reconnect/data loss issues in some corner cases). You've run into the problem that userspace simply doesn't check the return value from syscalls. It's not just scp, it's every program. Looking through cifs, you seem to do a lot of wait_event_interruptible() where you maybe should be doing wait_event_killable()? > ftruncate(3, 262144000) = ? ERESTARTSYS (To be > restarted if SA_RESTART is set) > --- SIGALRM {si_signo=SIGALRM, si_code=SI_KERNEL} --- > --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} --- > rt_sigreturn({mask=[ALRM]}) = 0 > ioctl(1, TIOCGWINSZ, {ws_row=51, ws_col=156, ws_xpixel=0, ws_ypixel=0}) = 0 > getpgrp() = 82563 Right ... so the code never calls ftruncate() again. Changing all of userspace is just not going to happen; maybe you could get stuff fixed in libc, but really ftruncate() should only be interrupted by a fatal signal and not by SIGALRM.