From patchwork Tue Feb 6 23:14:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 1895936 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=WcSQR0gd; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=147.75.199.223; helo=ny.mirrors.kernel.org; envelope-from=linux-cifs+bounces-1203-incoming=patchwork.ozlabs.org@vger.kernel.org; receiver=patchwork.ozlabs.org) Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org [147.75.199.223]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TTzc72s62z23gt for ; Wed, 7 Feb 2024 10:15:03 +1100 (AEDT) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 1E3AA1C23601 for ; Tue, 6 Feb 2024 23:15:01 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id ABA2A1C29B; Tue, 6 Feb 2024 23:14:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="WcSQR0gd" X-Original-To: linux-cifs@vger.kernel.org Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ADFA11CD13; Tue, 6 Feb 2024 23:14:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.51 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707261297; cv=none; b=ok0iuKQ8IgZHj3F18/d9vBYl4tGHhstc+0fVrHPEZ13RZjhrIOrEufvEmtZaL5uAE0P8NZZWXoPgxX7LGEqZPkw9BP7SFYV6c1lh/DFXNQc/91z6tFBs0QISs8wK6WppBfX8gbJrBpcBhDZJUEBTytQ8uhDa84nWlWPwseKu5mc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707261297; c=relaxed/simple; bh=/uEb6jR0llRxX5SjlEJDt+cYl6TgrmJHrp6Hr9lCEtc=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=oBupS2q0q3l16ojHBX/ycGDci5ddBNSE/ZW086bt8gI7Fy2EtZzdQ+G0ZyQjqyjVv3ncA6TqyJhLC/UGQSoMUZNu5RNLv9fcEeSoAhRSU11NWo8jDFXmnU2VEnCcozZ6RgOnJwpqBfVA722EK5VbiRFCaHkdT0TRnGrTXKkZzyw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=WcSQR0gd; arc=none smtp.client-ip=209.85.167.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-51165efb684so7343e87.3; Tue, 06 Feb 2024 15:14:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1707261294; x=1707866094; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=vjqghvpCB2iHALs069ZaG4OVOMOAjrIIrgdIKVWuNVo=; b=WcSQR0gd82lfd0xlFKYfz2UvQLmQMou4uvXKAPINBJivEXTCa77C8eRlLUPogBR+QS H0y0z8rhn7WZOifPhmR33lAsiocXrcx+BwgXEERCY8AK/yMJQi6bLLQ3CVJaQTco1sh9 xGXNGS+Le0Q6AxhcCEwrwZBWAoxs1+ct2ZJ8q/SaIwa//XNIbykVlFvQDeHjo0RaeTh/ MlfBTF56lz65T5ucD529HtBo8xpm45qWRlwX74oh7yrYPeH4kVPPaMa9lKw9PsPlL/7N wi/xPwW+RRJGRQAZJHjFF/DZuyBKvplr7zj4Aih8h3GZ8rvpdnlkDXhZA1CZosSaw3kB +oYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707261294; x=1707866094; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=vjqghvpCB2iHALs069ZaG4OVOMOAjrIIrgdIKVWuNVo=; b=ZniDwAN/lcQcc/CM//rc9hM1X2pWPEvPfoN55vwaerxi01tTv2AAmfG5vRNIDaAKIo xfU/ZHqlDc06L3Yb3faf6Hl9OF/zy1uh7CPUCt0cl3vpwfRuuXCh5JVPBzmxhpiwtfh3 lmHSdgkCQeQkbdPcK/eMHhvXANrqirbRJ8iHHSwsKYAsb7xhei8DepLZWboYzlrSDQhN v1QfJfN3Y5YjIK3pwq2blQdhyRcTbALtrwCyS/RYNRTzJxSGoUIajuIw7AaJjtxWKWn1 s5Qeqm5jTDAr0eC8wBVKrNAZl80/NXAqQS6CxZDZnLslQruC11OMTWD/aLtfvafBufOp 7AmA== X-Gm-Message-State: AOJu0Yy9Eax/jV0wVGLs3HTrDZN/90tRigfmkz3A7/G7YmBaLTRUWj44 7VzMYpgqV2OYUOTA5Z5OK/Tf9pNNNDPCEjbzcNo30gdILlLNYZz1L6SIgeXzl1gx06Uva35cCFh P5QJ48UstCPM1aze6OayReDwPVmZZEFffcKs= X-Google-Smtp-Source: AGHT+IHgpu+5FVFmjZnD51NP10VnrBf8XZMoPIiIk/uea2YJbAdLmFS/d6s4NFyshoMvEjf9M4Lzon1pgrNPMlfsj08= X-Received: by 2002:ac2:5211:0:b0:511:19b1:95b6 with SMTP id a17-20020ac25211000000b0051119b195b6mr2598935lfl.63.1707261293522; Tue, 06 Feb 2024 15:14:53 -0800 (PST) Precedence: bulk X-Mailing-List: linux-cifs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Steve French Date: Tue, 6 Feb 2024 17:14:42 -0600 Message-ID: Subject: [PATCH] fix netfs/folios regression To: David Howells , CIFS , linux-fsdevel Cc: Matthew Wilcox , ronnie sahlberg , "R. Diez" A case was recently reported where an old (SMB1) server negotiated a maximum write size that was not a multiple of PAGE_SIZE, and it caused easy to reproduce data corruptions on sequential writes (e.g. cp) for files that were bigger than maximum write size. This could also be reproduced by setting the optional mount parm ("wsize") to a non-standard value that was not a multiple of 4096. The problem was introduced in 6.3-rc1 or rc2 probably by patch: "cifs: Change the I/O paths to use an iterator rather than a page list" The code in question is a little hard to follow, and may eventually get rewritten by later folio/netfs patches from David Howells but the problem is in cifs_write_back_from_locked_folio() and cifs_writepages_region() where after the write (of maximum write size) completes, the next write skips to the beginning of the next page (leaving the tail end of the previous page unwritten). This is not an issue with typical servers and typical wsize values because those will almost always be a multiple of 4096, but in the bug report the server in question was old and had sent a value for maximum write size that was not a multiple of 4096. This can be a temporary fix, that can be removed as netfs/folios implementation improves here - but in the short term the easiest way to fix this seems to be to round the negotiated maximum_write_size down if not a multiple of 4096, to be a multiple of 4096 (this can be removed in the future when the folios code is found which caused this), and also warn the user if they pick a wsize that is not recommended, not a multiple of 4096. From 115f9424e2899269084069e88296b6481a0250a5 Mon Sep 17 00:00:00 2001 From: Steve French Date: Tue, 6 Feb 2024 16:34:22 -0600 Subject: [PATCH] smb: Fix regression in writes when non-standard maximum write size negotiated The conversion to netfs in the 6.3 kernel caused a regression when maximum write size is set by the server to an unexpected value which is not a multiple of 4096 (similarly if the user overrides the maximum write size by setting mount parm "wsize", but sets it to a value that is not a multiple of 4096). When negotiated write size is not a multiple of 4096 the netfs code can skip the end of the final page when doing large sequential writes causing data corruption. This section of code is being rewritten/removed due to a large netfs change, but until that point (from 6.3 kernel until now) we can not support non-standard maximum write sizes. Add a warning if a user specifies a wsize on mount that is not a multiple of 4096, and also add a change where we round down the maximum write size if the server negotiates a value that is not a multiple of 4096. Reported-by: R. Diez" Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list") Suggested-by: Ronnie Sahlberg Cc: stable@vger.kernel.org Cc: David Howells Signed-off-by: Steve French --- fs/smb/client/connect.c | 2 +- fs/smb/client/fs_context.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index c5cf88de32b7..9ceb3b2c614b 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3441,7 +3441,7 @@ int cifs_mount_get_tcon(struct cifs_mount_ctx *mnt_ctx) */ if ((cifs_sb->ctx->wsize == 0) || (cifs_sb->ctx->wsize > server->ops->negotiate_wsize(tcon, ctx))) - cifs_sb->ctx->wsize = server->ops->negotiate_wsize(tcon, ctx); + cifs_sb->ctx->wsize = round_down(server->ops->negotiate_wsize(tcon, ctx), 4096); if ((cifs_sb->ctx->rsize == 0) || (cifs_sb->ctx->rsize > server->ops->negotiate_rsize(tcon, ctx))) cifs_sb->ctx->rsize = server->ops->negotiate_rsize(tcon, ctx); diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 82eafe0815dc..55157778e553 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -1141,6 +1141,8 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, case Opt_wsize: ctx->wsize = result.uint_32; ctx->got_wsize = true; + if (round_up(ctx->wsize, 4096) != ctx->wsize) + cifs_dbg(VFS, "wsize should be a multiple of 4096\n"); break; case Opt_acregmax: ctx->acregmax = HZ * result.uint_32; -- 2.40.1