All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shyam Prasad N <nspmangalore@gmail.com>
To: Steve French <smfrench@gmail.com>
Cc: Enzo Matsumiya <ematsumiya@suse.de>,
	CIFS <linux-cifs@vger.kernel.org>, Paulo Alcantara <pc@cjr.nz>
Subject: Re: [PATCH] cifs: send workstation name during ntlmssp session setup
Date: Sat, 6 Nov 2021 09:08:05 +0530	[thread overview]
Message-ID: <CANT5p=qOQDg4xDbx6oZafJc+gnM0pN+aYOgjokU54ZeLXq_uDQ@mail.gmail.com> (raw)
In-Reply-To: <CAH2r5mvQG0DFmMdzojH2u_w2=_9oGRV++AnEt_d7WJzj=-uTKA@mail.gmail.com>

Thanks for the reviews, Paulo and Enzo. Please read my replies below:

On Sat, Nov 6, 2021 at 7:10 AM Steve French <smfrench@gmail.com> wrote:
>
> Interesting suggestions, probably worth experimenting with but even the original patch could help a lot eg help server to understand type of client connecting to it when debugging
>
> On Fri, Nov 5, 2021, 21:38 Enzo Matsumiya <ematsumiya@suse.de> wrote:
>>
>> Hi Shyam,
>>
>> I have some observations/suggestions regarding your patch:
>>
>> On 11/06, Shyam Prasad N wrote:
>> >Hi Steve,
>> >
>> >Please review this patch, and let me know what you think.
>> >Having this info in the workstation field of session setup helps
>> >server debugging in two ways.
>> >1. It helps identify the client by node name.
>> >2. It helps get the kernel release running on the client side.
>> >
>> >https://github.com/sprasad-microsoft/smb3-kernel-client/commit/d988e704dd9170c19ff94d9421c017e65dbbaac1.patch
>>
>> - AFAICS it doesn't consider runtime hostname changes. Is it important
>>    to keep track of it? Would changing it mid-auth steps break it
>>    somehow?
I think that's okay. AFAIK, that's only used for
debugging/troubleshooting purposes. So it doesn't need to be a 100%
accurate.

>>
>> - I didn't understand the purpose of CIFS_DEFAULT_WORKSTATION_NAME. Why
>>    not simply use utsname()->nodename? Or even init_utsname()->nodename, which
>>    is supposed to be always valid.
I initially did not have the utsname changes. That idea was an afterthought.
Sure. I'll update the patch to fix this.

>>
>> - Ditto for CIFS_MAX_WORKSTATION_LEN. utsname()->nodename has at most 65
>>    bytes (__NEW_UTS_LEN + 1) anyway. Perhaps using MAXHOSTNAMELEN from
>>    <asm/param.h> would be a more generic approach.
>>    (btw this is because nodename is the unqualified hostname, sans-domain)
Noted.

>>
>> - Instead of setting workstation_name to "nodename:release", why not
>>    implement the VERSION structure (MS-NLMP 2.2.2.10)? Then use
>>    LINUX_VERSION_* from <linux/version.h> or parse utsname()->release.
That's a good idea. Let me explore that too.

>>
>> >I ran some basic testing with the patch. Seems to serve the purpose.
>> >Please let me know if I'm missing something.
>>
>> I hope I didn't miss anything.
>>
>>
>> Cheers,
>>
>> Enzo



-- 
Regards,
Shyam

  parent reply	other threads:[~2021-11-06  3:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-05 19:22 [PATCH] cifs: send workstation name during ntlmssp session setup Shyam Prasad N
2021-11-06  1:38 ` Enzo Matsumiya
     [not found]   ` <CAH2r5mvQG0DFmMdzojH2u_w2=_9oGRV++AnEt_d7WJzj=-uTKA@mail.gmail.com>
2021-11-06  3:38     ` Shyam Prasad N [this message]
2021-11-06  7:02       ` Shyam Prasad N
2021-11-07 10:49       ` Stefan Metzmacher
2021-11-08  7:11         ` Shyam Prasad N

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANT5p=qOQDg4xDbx6oZafJc+gnM0pN+aYOgjokU54ZeLXq_uDQ@mail.gmail.com' \
    --to=nspmangalore@gmail.com \
    --cc=ematsumiya@suse.de \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@cjr.nz \
    --cc=smfrench@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.