| Message ID | 20210926135543.119127-1-linkinjeon@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ksmbd: a bunch of patches | expand |
Am 26.09.21 um 15:55 schrieb Namjae Jeon: > Cc: Tom Talpey <tom@talpey.com> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Böhme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Cc: Hyunchul Lee <hyc.lee@gmail.com> > Cc: Sergey Senozhatsky <senozhatsky@chromium.org> > > v2: > - update comments of smb2_get_data_area_len(). > - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). > - fix 32bit overflow in smb2_set_info. > > v3: > - add buffer check for ByteCount of smb negotiate request. > - Moved buffer check of to the top of loop to avoid unneeded behavior when > out_buf_len is smaller than network_interface_info_ioctl_rsp. > - get correct out_buf_len which doesn't exceed max stream protocol length. > - subtract single smb2_lock_element for correct buffer size check in > ksmbd_smb2_check_message(). looks like you haven't pushed those to github yet? At least I don't see an update to ksmbd-for-next-pending. I'd prefer fetching from git to review the patches. I also have a few patches on top. Thanks! -slow
2021-09-26 23:27 GMT+09:00, Ralph Boehme <slow@samba.org>: > Am 26.09.21 um 15:55 schrieb Namjae Jeon: >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Ralph Böhme <slow@samba.org> >> Cc: Steve French <smfrench@gmail.com> >> Cc: Hyunchul Lee <hyc.lee@gmail.com> >> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >> >> v2: >> - update comments of smb2_get_data_area_len(). >> - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). >> - fix 32bit overflow in smb2_set_info. >> >> v3: >> - add buffer check for ByteCount of smb negotiate request. >> - Moved buffer check of to the top of loop to avoid unneeded behavior >> when >> out_buf_len is smaller than network_interface_info_ioctl_rsp. >> - get correct out_buf_len which doesn't exceed max stream protocol >> length. >> - subtract single smb2_lock_element for correct buffer size check in >> ksmbd_smb2_check_message(). > > looks like you haven't pushed those to github yet? At least I don't see > an update to ksmbd-for-next-pending. > > I'd prefer fetching from git to review the patches. I also have a few > patches on top. Done, Please check it:) Thanks! > > Thanks! > -slow > > -- > Ralph Boehme, Samba Team https://samba.org/ > SerNet Samba Team Lead https://sernet.de/en/team-samba >
Hi Namjae Am 26.09.21 um 15:55 schrieb Namjae Jeon: > Cc: Tom Talpey <tom@talpey.com> > Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> > Cc: Ralph Böhme <slow@samba.org> > Cc: Steve French <smfrench@gmail.com> > Cc: Hyunchul Lee <hyc.lee@gmail.com> > Cc: Sergey Senozhatsky <senozhatsky@chromium.org> > > v2: > - update comments of smb2_get_data_area_len(). > - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). > - fix 32bit overflow in smb2_set_info. > > v3: > - add buffer check for ByteCount of smb negotiate request. > - Moved buffer check of to the top of loop to avoid unneeded behavior when > out_buf_len is smaller than network_interface_info_ioctl_rsp. > - get correct out_buf_len which doesn't exceed max stream protocol length. > - subtract single smb2_lock_element for correct buffer size check in > ksmbd_smb2_check_message(). I think there are a few issues with this patchset. I'm working on fixes and improvements and will push my branch to my github clone once I'm ready. I guess it's going to take a few days. Cheers! -slow
2021-09-28 0:42 GMT+09:00, Ralph Boehme <slow@samba.org>: > Hi Namjae Hi Ralph, > > Am 26.09.21 um 15:55 schrieb Namjae Jeon: >> Cc: Tom Talpey <tom@talpey.com> >> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >> Cc: Ralph Böhme <slow@samba.org> >> Cc: Steve French <smfrench@gmail.com> >> Cc: Hyunchul Lee <hyc.lee@gmail.com> >> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >> >> v2: >> - update comments of smb2_get_data_area_len(). >> - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). >> - fix 32bit overflow in smb2_set_info. >> >> v3: >> - add buffer check for ByteCount of smb negotiate request. >> - Moved buffer check of to the top of loop to avoid unneeded behavior >> when >> out_buf_len is smaller than network_interface_info_ioctl_rsp. >> - get correct out_buf_len which doesn't exceed max stream protocol >> length. >> - subtract single smb2_lock_element for correct buffer size check in >> ksmbd_smb2_check_message(). > > I think there are a few issues with this patchset. I'm working on fixes > and improvements and will push my branch to my github clone once I'm > ready. I guess it's going to take a few days. It sounds like you're making a patch based on this patch-set. If there is missing something for buffer check, You can add a patch on top of this, but if there are wrong codes in patch-set, It is right to leave a review comment to update this patch-set. Thanks! > > Cheers! > -slow > > -- > Ralph Boehme, Samba Team https://samba.org/ > SerNet Samba Team Lead https://sernet.de/en/team-samba >
Am 28.09.21 um 01:57 schrieb Namjae Jeon: > 2021-09-28 0:42 GMT+09:00, Ralph Boehme <slow@samba.org>: >> Hi Namjae > Hi Ralph, > >> >> Am 26.09.21 um 15:55 schrieb Namjae Jeon: >>> Cc: Tom Talpey <tom@talpey.com> >>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> >>> Cc: Ralph Böhme <slow@samba.org> >>> Cc: Steve French <smfrench@gmail.com> >>> Cc: Hyunchul Lee <hyc.lee@gmail.com> >>> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> >>> >>> v2: >>> - update comments of smb2_get_data_area_len(). >>> - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). >>> - fix 32bit overflow in smb2_set_info. >>> >>> v3: >>> - add buffer check for ByteCount of smb negotiate request. >>> - Moved buffer check of to the top of loop to avoid unneeded behavior >>> when >>> out_buf_len is smaller than network_interface_info_ioctl_rsp. >>> - get correct out_buf_len which doesn't exceed max stream protocol >>> length. >>> - subtract single smb2_lock_element for correct buffer size check in >>> ksmbd_smb2_check_message(). >> >> I think there are a few issues with this patchset. I'm working on fixes >> and improvements and will push my branch to my github clone once I'm >> ready. I guess it's going to take a few days. > It sounds like you're making a patch based on this patch-set. If there > is missing something for buffer check, You can add a patch on top of > this, but if there are wrong codes in patch-set, It is right to leave > a review comment to update this patch-set. both: there are issues with the patch and I have changes on-top. :) It just takes a bit of time due to other stuff going on currently like SDC. -slow
Am 28.09.21 um 05:26 schrieb Ralph Boehme: > both: there are issues with the patch and I have changes on-top. :) It > just takes a bit of time due to other stuff going on currently like SDC. finally... :) Please check my branch <https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> for added commits and two SQUASHes. Remaining commits reviewed-by: me. Oh, and I also split out the setinfo basic infolevel changes into its own commit. Let me know what you think of the additional checks I've added. Cheers! -slow
2021-09-28 22:43 GMT+09:00, Ralph Boehme <slow@samba.org>: > Am 28.09.21 um 05:26 schrieb Ralph Boehme: >> both: there are issues with the patch and I have changes on-top. :) It >> just takes a bit of time due to other stuff going on currently like SDC. > > finally... :) > > Please check my branch > <https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> > > for added commits and two SQUASHes. Remaining commits reviewed-by: me. Yep, looks good, I will update them in patches. And thanks for your review! > > Oh, and I also split out the setinfo basic infolevel changes into its > own commit. If you want to add clean-up patch first, we can change get_file_basic_info() together in patch. I will update it also. > > Let me know what you think of the additional checks I've added. You should submit patches to the list to be checked by other developers. Thanks! > > Cheers! > -slow > > -- > Ralph Boehme, Samba Team https://samba.org/ > SerNet Samba Team Lead https://sernet.de/en/team-samba >
Am 28.09.21 um 16:23 schrieb Namjae Jeon: > 2021-09-28 22:43 GMT+09:00, Ralph Boehme <slow@samba.org>: >> Am 28.09.21 um 05:26 schrieb Ralph Boehme: >>> both: there are issues with the patch and I have changes on-top. :) It >>> just takes a bit of time due to other stuff going on currently like SDC. >> >> finally... :) >> >> Please check my branch >> <https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> >> >> for added commits and two SQUASHes. Remaining commits reviewed-by: me. > Yep, looks good, I will update them in patches. And thanks for your review! thanks! >> Oh, and I also split out the setinfo basic infolevel changes into its >> own commit. > If you want to add clean-up patch first, we can change > get_file_basic_info() together in patch. I will update it also. >> >> Let me know what you think of the additional checks I've added. > You should submit patches to the list to be checked by other developers. everyone can fetch from that branch. And as I'm not merely doing patch review, but am changing, expanding, fixing patches, an email patch workflow doesn't work. Once the patchset has stabilized enough, it can go to the list. Thanks! -slow
On Tue, Sep 28, 2021 at 06:33:46PM +0200, Ralph Boehme wrote: >Am 28.09.21 um 16:23 schrieb Namjae Jeon: >>2021-09-28 22:43 GMT+09:00, Ralph Boehme <slow@samba.org>: >>>Am 28.09.21 um 05:26 schrieb Ralph Boehme: >>>>both: there are issues with the patch and I have changes on-top. :) It >>>>just takes a bit of time due to other stuff going on currently like SDC. >>> >>>finally... :) >>> >>>Please check my branch >>><https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> >>> >>>for added commits and two SQUASHes. Remaining commits reviewed-by: me. >>Yep, looks good, I will update them in patches. And thanks for your review! > >thanks! > >>>Oh, and I also split out the setinfo basic infolevel changes into its >>>own commit. >>If you want to add clean-up patch first, we can change >>get_file_basic_info() together in patch. I will update it also. >>> >>>Let me know what you think of the additional checks I've added. >>You should submit patches to the list to be checked by other developers. > >everyone can fetch from that branch. And as I'm not merely doing patch >review, but am changing, expanding, fixing patches, an email patch >workflow doesn't work. +1 on this. email-based patch workflow is fine when patches don't go through many iterations or have many people working on them, but when those things are true a repository-based workflow is far better (IMHO). Everyone knows how to use git (these days :-). It would be good to get to the point where the list is used as a "release management" tool where patches that have already been completely reviewed and signed-off are sent and archived.
2021-09-28 23:23 GMT+09:00, Namjae Jeon <linkinjeon@kernel.org>: > 2021-09-28 22:43 GMT+09:00, Ralph Boehme <slow@samba.org>: >> Am 28.09.21 um 05:26 schrieb Ralph Boehme: >>> both: there are issues with the patch and I have changes on-top. :) It >>> just takes a bit of time due to other stuff going on currently like SDC. >> >> finally... :) >> >> Please check my branch >> <https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> >> >> for added commits and two SQUASHes. Remaining commits reviewed-by: me. > Yep, looks good, I will update them in patches. And thanks for your review! When I take a look, I found issues in two squashes. I leave comments... I still prefer you give review comments on patches in the list. >> >> Oh, and I also split out the setinfo basic infolevel changes into its >> own commit. > If you want to add clean-up patch first, we can change > get_file_basic_info() together in patch. I will update it also. >> >> Let me know what you think of the additional checks I've added. > You should submit patches to the list to be checked by other developers. > > Thanks! >> >> Cheers! >> -slow >> >> -- >> Ralph Boehme, Samba Team https://samba.org/ >> SerNet Samba Team Lead https://sernet.de/en/team-samba >> >
On 9/28/2021 1:33 PM, Jeremy Allison wrote: > On Tue, Sep 28, 2021 at 06:33:46PM +0200, Ralph Boehme wrote: >> Am 28.09.21 um 16:23 schrieb Namjae Jeon: >>> 2021-09-28 22:43 GMT+09:00, Ralph Boehme <slow@samba.org>: >>>> Am 28.09.21 um 05:26 schrieb Ralph Boehme: >>>>> both: there are issues with the patch and I have changes on-top. :) It >>>>> just takes a bit of time due to other stuff going on currently like >>>>> SDC. >>>> >>>> finally... :) >>>> >>>> Please check my branch >>>> <https://github.com/slowfranklin/smb3-kernel/commits/ksmbd-for-next-pending> >>>> >>>> >>>> for added commits and two SQUASHes. Remaining commits reviewed-by: me. >>> Yep, looks good, I will update them in patches. And thanks for your >>> review! >> >> thanks! >> >>>> Oh, and I also split out the setinfo basic infolevel changes into its >>>> own commit. >>> If you want to add clean-up patch first, we can change >>> get_file_basic_info() together in patch. I will update it also. >>>> >>>> Let me know what you think of the additional checks I've added. >>> You should submit patches to the list to be checked by other developers. >> >> everyone can fetch from that branch. And as I'm not merely doing patch >> review, but am changing, expanding, fixing patches, an email patch >> workflow doesn't work. > > +1 on this. email-based patch workflow is fine when patches > don't go through many iterations or have many people working > on them, but when those things are true a repository-based > workflow is far better (IMHO). Everyone knows how to use > git (these days :-). I completely agree that email is inefficient, but git is a terrible way to have a discussion. We should attempt to be sure we have those, and that everybody has a chance to see the proposals without having to go to the web five times a day. Please take this as a request for regular git-send-email updates to this list, so I can see them if I'm not online. Maybe add a boilerplate line to direct to the git repo webui. I'm sure a few others will appreciate it too. Tom. > It would be good to get to the point where the list is > used as a "release management" tool where patches that > have already been completely reviewed and signed-off > are sent and archived. >
On Wed, Sep 29, 2021 at 11:28:09AM -0400, Tom Talpey wrote: > >I completely agree that email is inefficient, but git is a terrible >way to have a discussion. We should attempt to be sure we have >those, and that everybody has a chance to see the proposals without >having to go to the web five times a day. > >Please take this as a request for regular git-send-email updates to >this list, so I can see them if I'm not online. Maybe add a boilerplate >line to direct to the git repo webui. I'm sure a few others will >appreciate it too. Samba does well with the web-based discussion mechanism around merge-requests (MR's) in gitlab. I assume github has something similar. Maybe send the initial patch to the list with a link to the github MR so people interested in reviewing/discussing can follow along there ?
Am 29.09.21 um 17:42 schrieb Jeremy Allison: > On Wed, Sep 29, 2021 at 11:28:09AM -0400, Tom Talpey wrote: >> >> I completely agree that email is inefficient, but git is a terrible >> way to have a discussion. We should attempt to be sure we have >> those, and that everybody has a chance to see the proposals without >> having to go to the web five times a day. >> >> Please take this as a request for regular git-send-email updates to >> this list, so I can see them if I'm not online. Maybe add a boilerplate >> line to direct to the git repo webui. I'm sure a few others will >> appreciate it too. > > Samba does well with the web-based discussion mechanism > around merge-requests (MR's) in gitlab. I assume github > has something similar. > > Maybe send the initial patch to the list with a link > to the github MR so people interested in reviewing/discussing > can follow along there ? well, if I could have it the way I wanted, then this would be it. But I understand that adopting new workflows is not something I can impose -- at least not without paying for an insane amount of Lakritz-Gitarren that I tend to use to bribe metze into doing something I want him to do. :) The problem is not so much doing the *review* on patches sent to the list. While Samba has moved away from doing review on patch emails, it can certainly be done. The point is, once you go beyond "review" by taking someone else's patchset, modifying it deeply, reordering patches, adding patches, rewriting patches, dropping patches and so on, that's when the patchset-as-email workflow explodes and coordination via git is needed. Once such a collaboratively worked on patchset stabilizes, it can of course again go to the mailing list. -slow
On 9/29/2021 12:38 PM, Ralph Boehme wrote: > Am 29.09.21 um 17:42 schrieb Jeremy Allison: >> On Wed, Sep 29, 2021 at 11:28:09AM -0400, Tom Talpey wrote: >>> >>> I completely agree that email is inefficient, but git is a terrible >>> way to have a discussion. We should attempt to be sure we have >>> those, and that everybody has a chance to see the proposals without >>> having to go to the web five times a day. >>> >>> Please take this as a request for regular git-send-email updates to >>> this list, so I can see them if I'm not online. Maybe add a boilerplate >>> line to direct to the git repo webui. I'm sure a few others will >>> appreciate it too. >> >> Samba does well with the web-based discussion mechanism >> around merge-requests (MR's) in gitlab. I assume github >> has something similar. >> >> Maybe send the initial patch to the list with a link >> to the github MR so people interested in reviewing/discussing >> can follow along there ? > > well, if I could have it the way I wanted, then this would be it. But I > understand that adopting new workflows is not something I can impose -- > at least not without paying for an insane amount of Lakritz-Gitarren > that I tend to use to bribe metze into doing something I want him to do. :) I'm in for github if you send me some too! https://www.gutschmecker.com/produkt/haevy-metal-salzige-gitarren-10-x-150-g-tuete/ > The problem is not so much doing the *review* on patches sent to the > list. While Samba has moved away from doing review on patch emails, it > can certainly be done. Clearly, this effort bridges the Linux and Samba processes. We can definitely try. I guess I'm going to take some convincing. Tom. > The point is, once you go beyond "review" by taking someone else's > patchset, modifying it deeply, reordering patches, adding patches, > rewriting patches, dropping patches and so on, that's when the > patchset-as-email workflow explodes and coordination via git is needed. > > Once such a collaboratively worked on patchset stabilizes, it can of > course again go to the mailing list. > > -slow >
Am 29.09.21 um 18:45 schrieb Tom Talpey: > On 9/29/2021 12:38 PM, Ralph Boehme wrote: >> well, if I could have it the way I wanted, then this would be it. But >> I understand that adopting new workflows is not something I can impose >> -- at least not without paying for an insane amount of >> Lakritz-Gitarren that I tend to use to bribe metze into doing >> something I want him to do. :) > > I'm in for github if you send me some too! > > https://www.gutschmecker.com/produkt/haevy-metal-salzige-gitarren-10-x-150-g-tuete/ rofl I have to bookmark this, looks like one of those would buy me at least a 100-reviews-granted package from metze. :))) -slow
On Wed, Sep 29, 2021 at 11:45 AM Tom Talpey <tom@talpey.com> wrote: > > On 9/29/2021 12:38 PM, Ralph Boehme wrote: > > Am 29.09.21 um 17:42 schrieb Jeremy Allison: > >> On Wed, Sep 29, 2021 at 11:28:09AM -0400, Tom Talpey wrote: > >>> > >>> I completely agree that email is inefficient, but git is a terrible > >>> way to have a discussion. Agreed > >> Maybe send the initial patch to the list with a link > >> to the github MR so people interested in reviewing/discussing > >> can follow along there ? > > > > well, if I could have it the way I wanted, then this would be it. But I > > understand that adopting new workflows is not something I can impose -- > > at least not without paying for an insane amount of Lakritz-Gitarren > > that I tend to use to bribe metze into doing something I want him to do. :) > > I'm in for github if you send me some too! gitnub is fine for many things, and we can automated "kernel development process" things presumably with github easier than alternatives: - running "scripts/checkpatch" - make with C=1 and "_CHECK_ENDIAN" support turned on - kick off smbtorture tests (as Namjae already does in his branches in github) BUT ... we have to ensure a couple things. - we don't annoy Linus (and linux-next and stable maintainers) by doing things like web merges in github (he complained about the meaningless/distracting github web ui empty merge messages) - we don't miss reviewers in Linux who also want to see them on mailing lists (presumably some of fsdevel - ie more general patches that other fs developers outside smb world need to comment on) > > > > Once such a collaboratively worked on patchset stabilizes, it can of > > course again go to the mailing list. Makes sense
Am 29.09.21 um 19:11 schrieb Steve French: > gitnub is fine for many things, and we can automated "kernel > development process" > things presumably with github easier than alternatives: > - running "scripts/checkpatch" > - make with C=1 and "_CHECK_ENDIAN" support turned on > - kick off smbtorture tests (as Namjae already does in his branches in github) you can also add "doing review". :) As for running tests: I want that as well! :) How can I get that? Maybe other want to run CI on their patches too before posting them. > BUT ... we have to ensure a couple things. > - we don't annoy Linus (and linux-next and stable maintainers) by doing things > like web merges in github (he complained about the meaningless/distracting > github web ui empty merge messages) as said before: just don't do the merge there, just the review. That's the way Samba has been doing it for years. Are you actually aware of the current Samba workflow? -slow
2021-09-30 2:18 GMT+09:00, Ralph Boehme <slow@samba.org>: > Am 29.09.21 um 19:11 schrieb Steve French: >> gitnub is fine for many things, and we can automated "kernel >> development process" >> things presumably with github easier than alternatives: >> - running "scripts/checkpatch" >> - make with C=1 and "_CHECK_ENDIAN" support turned on >> - kick off smbtorture tests (as Namjae already does in his branches in >> github) > > you can also add "doing review". :) > > As for running tests: I want that as well! :) How can I get that? Maybe > other want to run CI on their patches too before posting them. > >> BUT ... we have to ensure a couple things. >> - we don't annoy Linus (and linux-next and stable maintainers) by doing >> things >> like web merges in github (he complained about the >> meaningless/distracting >> github web ui empty merge messages) > > as said before: just don't do the merge there, just the review. That's > the way Samba has been doing it for years. Are you actually aware of the > current Samba workflow? Is it friendly to new developers? I know samba workflow now too. New developers can do everything easily by simply subscribing to the mailing list. And do we review only the SMB protocol on github? If we review and discuss kernel common code usage and touching, it should be visible to the component kernel maintainers as well. And is the review history likely to be discarded on github? Doesn't it get thrown away the moment you change or update a patch? Also, review discussions left on each individual's github cannot be easily searched like mailing list. > > -slow > > -- > Ralph Boehme, Samba Team https://samba.org/ > SerNet Samba Team Lead https://sernet.de/en/team-samba >
2021년 9월 30일 (목) 오전 9:32, Namjae Jeon <linkinjeon@kernel.org>님이 작성: > > 2021-09-30 2:18 GMT+09:00, Ralph Boehme <slow@samba.org>: > > Am 29.09.21 um 19:11 schrieb Steve French: > >> gitnub is fine for many things, and we can automated "kernel > >> development process" > >> things presumably with github easier than alternatives: > >> - running "scripts/checkpatch" > >> - make with C=1 and "_CHECK_ENDIAN" support turned on > >> - kick off smbtorture tests (as Namjae already does in his branches in > >> github) > > > > you can also add "doing review". :) > > > > As for running tests: I want that as well! :) How can I get that? Maybe > > other want to run CI on their patches too before posting them. > > > >> BUT ... we have to ensure a couple things. > >> - we don't annoy Linus (and linux-next and stable maintainers) by doing > >> things > >> like web merges in github (he complained about the > >> meaningless/distracting > >> github web ui empty merge messages) > > > > as said before: just don't do the merge there, just the review. That's > > the way Samba has been doing it for years. Are you actually aware of the > > current Samba workflow? > Is it friendly to new developers? I know samba workflow now too. New > developers can do everything easily by simply subscribing to the > mailing list. And do we review only the SMB protocol on github? If we > review and discuss kernel common code usage and touching, it should be > visible to the component kernel maintainers as well. > I agreed. Kernel developers are familiar with reviews in the mailing list. And only for patches about the SMB protocol, If someone asks for review in github, we can do it. > And is the review history likely to be discarded on github? Doesn't it > get thrown away the moment you change or update a patch? Also, review > discussions left on each individual's github cannot be easily searched > like mailing list. > > > > -slow > > > > -- > > Ralph Boehme, Samba Team https://samba.org/ > > SerNet Samba Team Lead https://sernet.de/en/team-samba > >
Am 30.09.21 um 02:32 schrieb Namjae Jeon: > 2021-09-30 2:18 GMT+09:00, Ralph Boehme <slow@samba.org>: >> as said before: just don't do the merge there, just the review. That's >> the way Samba has been doing it for years. Are you actually aware of the >> current Samba workflow? > Is it friendly to new developers? yes. It's just a different tooling you have to wrap your head around. In the case of Samba it has the added benefit, that every interested contributor can make use of the full Samba CI. This works by using a shared repo on gitlab where every registered developer (so you have to ask to be added, but this is freely granted) where you push your patchset to a branch name prefixed with your username (to avoid stepping on someone else's branch). That triggers an automated *full* CI run, so new contributors can be sure not to waste time of other developers before asking for review. > I know samba workflow now too. New > developers can do everything easily by simply subscribing to the > mailing list. Sure, instead of pushing the resulting patchset from the review of your patchset to a github branch, I could send my patchset to the ML. Having now used a more a different workflow for a few years, I appreciate how much more natural it feels to share, work on and review code with a tooling that is much more intergrated to git. So what I'd like to propose for now is let's stick to the ML for now, next time I will send my patchset to the ML instead, but let's also consider and maybe test different possible toolings. > And do we review only the SMB protocol on github? If we > review and discuss kernel common code usage and touching, it should be > visible to the component kernel maintainers as well. > > And is the review history likely to be discarded on github? I don't think so, I'm not 100% sure on this though. But as it's NOT discarded on gitlab, I guess github will match feature wise. > Doesn't it > get thrown away the moment you change or update a patch? Also, review > discussions left on each individual's github cannot be easily searched > like mailing list. Yeah, but does that really matter? So far this was not missed in the Samba gitlab tooling. -slow
Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> v2: - update comments of smb2_get_data_area_len(). - fix wrong buffer size check in fsctl_query_iface_info_ioctl(). - fix 32bit overflow in smb2_set_info. v3: - add buffer check for ByteCount of smb negotiate request. - Moved buffer check of to the top of loop to avoid unneeded behavior when out_buf_len is smaller than network_interface_info_ioctl_rsp. - get correct out_buf_len which doesn't exceed max stream protocol length. - subtract single smb2_lock_element for correct buffer size check in ksmbd_smb2_check_message(). Namjae Jeon (5): ksmbd: add the check to vaildate if stream protocol length exceeds maximum value ksmbd: add validation in smb2_ioctl ksmbd: add request buffer validation in smb2_set_info ksmbd: check strictly data area in ksmbd_smb2_check_message() ksmbd: add validation in smb2 negotiate fs/ksmbd/connection.c | 10 +- fs/ksmbd/smb2misc.c | 98 +++++++------- fs/ksmbd/smb2pdu.c | 295 ++++++++++++++++++++++++++++++++---------- fs/ksmbd/smb2pdu.h | 9 ++ fs/ksmbd/smb_common.c | 38 ++++-- fs/ksmbd/smb_common.h | 4 +- fs/ksmbd/vfs.c | 2 +- fs/ksmbd/vfs.h | 2 +- 8 files changed, 321 insertions(+), 137 deletions(-)