Bug 4505 - Bug in irix_oplock_receive_message()
Summary: Bug in irix_oplock_receive_message()
Status: NEW
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.0.25
Hardware: SGI IRIX
: P3 normal
Target Milestone: none
Assignee: Samba Bugzilla Account
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-04-11 13:23 UTC by Alex Elder
Modified: 2007-04-16 07:18 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Elder 2007-04-11 13:23:24 UTC
I found this by reviewing the code; I haven't verified it.

In oplock_irix.c:irix_oplock_receive_message(), there is a line that
looks like this:
    if(fcntl(oplock_pipe_read, F_OPLKSTAT, &os) < 0) {

The first argument to the fcntl() call appears to be wrong.
It should be oplock_pipe_write.  I.e., I believe to work
correctly it should look like this:
    if(fcntl(oplock_pipe_write, F_OPLKSTAT, &os) < 0) {
Comment 1 Volker Lendecke 2007-04-11 13:32:06 UTC
A very similar code line is even back in 3.0.14a before the big rewrite started. So this is ancient.

Can someone with the ability to test this try to confirm it? I can't really verify it because I don't have an IRIX box handy.

Volker
Comment 2 Alex Elder 2007-04-13 15:43:43 UTC
I did a test under Irix, not of Samba but of the fcntl() calls
Samba makes.  I found that either the read side or the write
side of the pipe works on the F_OPLKSTAT request.  This happens
to work because of how Irix implements pipes--namely that both
ends share the same vnode.

Still, the Samba code *is* wrong, even though it will exhibit
correct behavior as written.
Comment 3 Gerald (Jerry) Carter (dead mail address) 2007-04-14 10:51:25 UTC
Alex, none of the developers have ready access to IRIX boxes
so it is very difficult to very your results.  Can you produce 
a test case where Samba fails due to this bug and prove that 
your changes fixes the problem?
Comment 4 Alex Elder 2007-04-16 07:04:45 UTC
My last update was based on running a test program
that narrows down the issue considerably, and doesn't
actually execute the Samba code itself.  Instead, it
just does the sequence of fcntl() calls that Samba
makes, in isolation.

My test program verifies that what Samba is doing *does*
work correctly.  However the code is still technically
wrong because it is supplying the wrong side of the pipe
descriptor to the F_OPLKSTAT call.

I really don't do anything with Samba; I came across
this because I'm looking at oplocks at SGI.  I happened
to notice this problem so I reported it.

I have confidence that if you implement the change it
will correct the code but not change the underlying
behavior.

Or if you like, you can leave it; at this point it's
more of a theoretical problem.  It will only bite you
if SGI changes how Irix handles pipes, which at this
point is very unlikely.
Comment 5 Volker Lendecke 2007-04-16 07:10:31 UTC
I'd feel better with that change if you could point us at some docs that tell us how IRIX oplocks work.

Volker
Comment 6 Alex Elder 2007-04-16 07:18:53 UTC
This is from the fcntl() man page.  I got it on a google search
of F_OPLKSTAT.  It's on an SGI site that I'm pretty sure should
be public.  In any case, I'll paste below the relevent chunk
regarding F_OPLKSTAT.  If you want, I can send you more of the
man page, or my test program.

    http://tinyurl.com/2a44yn

     F_OPLKSTAT
               The oplock state change command is used to get state change
               information on any recently externally referenced files
               registered with the given write side pipe (eg p[1] from a
               pipe(int *p) call).  The returned oplock_stat_t structure
               pointed at by arg contains the current state (os_state) and the
               dev/ino information (os_dev/os_ino) to identify the file.

               This is only done on the write side of a pipe for which
               select() indicates there is a byte of data to read() on the
               read side.  A byte of data must then be read() from the read
               side of the pipe for each successful F_OPLKSTAT run on the
               write side for select() to again give proper notification.

               External references that cause state change notification will
               hang for a while until the SMB server acknowledges the
               revocation (typically after revoking the oplock it granted to
               the SMB client) or until the systunable oplock_timeout expires.