2017-07-14 16:42:53

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH 3.18 36/68] Handle mismatched open calls

On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> 3.18-stable review patch.  If anyone has any objections, please let me know.
>
> ------------------
>
> From: Sachin Prabhu <[email protected]>
>
> commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
[...]
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
>   .clear_stats = smb2_clear_stats,
>   .print_stats = smb2_print_stats,
>   .is_oplock_break = smb2_is_valid_oplock_break,
> + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> + .handle_cancelled_mid = smb2_handle_cancelled_mid,
>   .downgrade_oplock = smb2_downgrade_oplock,
>   .need_neg = smb2_need_neg,
>   .negotiate = smb2_negotiate,
> @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
>   .print_stats = smb2_print_stats,
>   .dump_share_caps = smb2_dump_share_caps,
>   .is_oplock_break = smb2_is_valid_oplock_break,
> + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> + .handle_cancelled_mid = smb2_handle_cancelled_mid,
>   .downgrade_oplock = smb2_downgrade_oplock,
>   .need_neg = smb2_need_neg,
>   .negotiate = smb2_negotiate,
[...]

This doesn't look right. handle_cancelled_mid should be initialised
once in each of the 3 smb_version_operations structures, shouldn't it?

Ben.

--
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part

2017-07-19 18:39:17

by Pavel Shilovskiy

[permalink] [raw]
Subject: RE: [PATCH 3.18 36/68] Handle mismatched open calls

2017-07-14 9:43 Ben Hutchings <[email protected]>:
> On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> > 3.18-stable review patch.  If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Sachin Prabhu <[email protected]>
> >
> > commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
> [...]
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
> >   .clear_stats = smb2_clear_stats,
> >   .print_stats = smb2_print_stats,
> >   .is_oplock_break = smb2_is_valid_oplock_break,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >   .downgrade_oplock = smb2_downgrade_oplock,
> >   .need_neg = smb2_need_neg,
> >   .negotiate = smb2_negotiate,
> > @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
> >   .print_stats = smb2_print_stats,
> >   .dump_share_caps = smb2_dump_share_caps,
> >   .is_oplock_break = smb2_is_valid_oplock_break,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> >   .downgrade_oplock = smb2_downgrade_oplock,
> >   .need_neg = smb2_need_neg,
> >   .negotiate = smb2_negotiate,
> [...]
>
> This doesn't look right. handle_cancelled_mid should be initialised once in each of the 3 smb_version_operations structures, shouldn't it?
>
> Ben.

Yes, you are right. Thanks for pointing it out.

Greg, I provided the patch to fix the above bug (see the attachment). Could you please look at it and apply to the 3.18.x kernel if it is suitable?

Best regards,
Pavel Shilovsky


Attachments:
0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch (1.84 kB)
0001-CIFS-Fix-handle_cancelled_mid-callback-initializatio.patch

2017-07-22 13:12:31

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3.18 36/68] Handle mismatched open calls

On Wed, Jul 19, 2017 at 06:39:13PM +0000, Pavel Shilovskiy wrote:
> 2017-07-14 9:43 Ben Hutchings <[email protected]>:
> > On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> > > 3.18-stable review patch.??If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Sachin Prabhu <[email protected]>
> > >
> > > commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
> > [...]
> > > --- a/fs/cifs/smb2ops.c
> > > +++ b/fs/cifs/smb2ops.c
> > > @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
> > > ? .clear_stats = smb2_clear_stats,
> > > ? .print_stats = smb2_print_stats,
> > > ? .is_oplock_break = smb2_is_valid_oplock_break,
> > > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > > ? .downgrade_oplock = smb2_downgrade_oplock,
> > > ? .need_neg = smb2_need_neg,
> > > ? .negotiate = smb2_negotiate,
> > > @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
> > > ? .print_stats = smb2_print_stats,
> > > ? .dump_share_caps = smb2_dump_share_caps,
> > > ? .is_oplock_break = smb2_is_valid_oplock_break,
> > > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > > ? .downgrade_oplock = smb2_downgrade_oplock,
> > > ? .need_neg = smb2_need_neg,
> > > ? .negotiate = smb2_negotiate,
> > [...]
> >
> > This doesn't look right. handle_cancelled_mid should be initialised once in each of the 3 smb_version_operations structures, shouldn't it?
> >
> > Ben.
>
> Yes, you are right. Thanks for pointing it out.
>
> Greg, I provided the patch to fix the above bug (see the attachment).
> Could you please look at it and apply to the 3.18.x kernel if it is
> suitable?

Looks good, now queued up, thanks.

greg k-h

2017-11-15 11:04:39

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3.18 36/68] Handle mismatched open calls

On Fri, Jul 14, 2017 at 05:42:32PM +0100, Ben Hutchings wrote:
> On Fri, 2017-05-05 at 11:32 -0700, Greg Kroah-Hartman wrote:
> > 3.18-stable review patch.��If anyone has any objections, please let me know.
> >
> > ------------------
> >
> > From: Sachin Prabhu <[email protected]>
> >
> > commit 38bd49064a1ecb67baad33598e3d824448ab11ec upstream.
> [...]
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -1461,6 +1461,8 @@ struct smb_version_operations smb21_oper
> > � .clear_stats = smb2_clear_stats,
> > � .print_stats = smb2_print_stats,
> > � .is_oplock_break = smb2_is_valid_oplock_break,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > � .downgrade_oplock = smb2_downgrade_oplock,
> > � .need_neg = smb2_need_neg,
> > � .negotiate = smb2_negotiate,
> > @@ -1542,6 +1544,8 @@ struct smb_version_operations smb30_oper
> > � .print_stats = smb2_print_stats,
> > � .dump_share_caps = smb2_dump_share_caps,
> > � .is_oplock_break = smb2_is_valid_oplock_break,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > + .handle_cancelled_mid = smb2_handle_cancelled_mid,
> > � .downgrade_oplock = smb2_downgrade_oplock,
> > � .need_neg = smb2_need_neg,
> > � .negotiate = smb2_negotiate,
> [...]
>
> This doesn't look right. handle_cancelled_mid should be initialised
> once in each of the 3 smb_version_operations structures, shouldn't it?

Yeah, something is odd with this, I'll look into it later today, thanks
for flagging it.

greg k-h

From 1573628596378860071@xxx Sat Jul 22 13:12:55 +0000 2017
X-GM-THRID: 1572917095851905670
X-Gmail-Labels: Inbox,Category Forums